From 252548cbd5256b83c764088a6b00acf3019b5adb Mon Sep 17 00:00:00 2001 From: nyoungbq Date: Wed, 27 Mar 2024 16:52:24 -0400 Subject: [PATCH 1/9] Update ITKImportFijiMontage.cpp, DataObject.cpp, DataObject.hpp, and 2 more files --- .../Algorithms/ITKImportFijiMontage.cpp | 2 +- src/simplnx/DataStructure/DataObject.cpp | 14 +++++----- src/simplnx/DataStructure/DataObject.hpp | 4 +-- .../Filter/Actions/RenameDataAction.cpp | 28 +++++++++++++++++-- .../Filter/Actions/RenameDataAction.hpp | 9 +++++- 5 files changed, 44 insertions(+), 13 deletions(-) diff --git a/src/Plugins/ITKImageProcessing/src/ITKImageProcessing/Filters/Algorithms/ITKImportFijiMontage.cpp b/src/Plugins/ITKImageProcessing/src/ITKImageProcessing/Filters/Algorithms/ITKImportFijiMontage.cpp index 4da20e4e33..2b87ce2ec2 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()) != 1) { return MakeErrorResult(-18543, fmt::format("Unable to rename the grayscale array to {}", imageDataPath.getTargetName())); } diff --git a/src/simplnx/DataStructure/DataObject.cpp b/src/simplnx/DataStructure/DataObject.cpp index 3d8823774f..46a730a552 100644 --- a/src/simplnx/DataStructure/DataObject.cpp +++ b/src/simplnx/DataStructure/DataObject.cpp @@ -162,38 +162,38 @@ std::string DataObject::getName() const return m_Name; } -bool DataObject::canRename(const std::string& name) const +int DataObject::canRename(const std::string& name) const { if(name == getName()) { - return true; + return 1; } if(!IsValidName(name)) { - return false; + return 0; } const auto* dataStructPtr = getDataStructure(); if(dataStructPtr == nullptr) { - return false; + return 0; } - return !std::any_of(m_ParentList.cbegin(), m_ParentList.cend(), [dataStructPtr, name](IdType parentId) { + return (std::any_of(m_ParentList.cbegin(), m_ParentList.cend(), [dataStructPtr, name](IdType parentId) { const auto* baseGroupPtr = dataStructPtr->getDataAs(parentId); if(baseGroupPtr == nullptr) { std::cout << "DataObject::canRename(name=" << name << ") cannot get baseGroup from parentId = " << parentId << std::endl; } return baseGroupPtr != nullptr && baseGroupPtr->contains(name); - }); + })) ? 2 : 0; } bool DataObject::rename(const std::string& name) { - if(!canRename(name)) + if(canRename(name) != 1) { return false; } diff --git a/src/simplnx/DataStructure/DataObject.hpp b/src/simplnx/DataStructure/DataObject.hpp index adc004913c..387671713f 100644 --- a/src/simplnx/DataStructure/DataObject.hpp +++ b/src/simplnx/DataStructure/DataObject.hpp @@ -205,9 +205,9 @@ class SIMPLNX_EXPORT DataObject * @brief Checks and returns if the DataObject can be renamed to the provided * value. * @param name - * @return bool + * @return int: 0 = false, 1 = true, 2 = duplicate object found */ - bool canRename(const std::string& name) const; + int canRename(const std::string& name) const; /** * @brief Attempts to rename the DataObject to the provided value. diff --git a/src/simplnx/Filter/Actions/RenameDataAction.cpp b/src/simplnx/Filter/Actions/RenameDataAction.cpp index e63f35d50d..158ca7a94b 100644 --- a/src/simplnx/Filter/Actions/RenameDataAction.cpp +++ b/src/simplnx/Filter/Actions/RenameDataAction.cpp @@ -9,9 +9,10 @@ using namespace nx::core; 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,11 +28,28 @@ Result<> RenameDataAction::apply(DataStructure& dataStructure, Mode mode) const return MakeErrorResult(-6601, ss); } - if(!dataObject->canRename(m_NewName)) + int validRename = dataObject->canRename(m_NewName); + if(validRename == 0) { std::string ss = fmt::format("{}Could not rename DataObject at '{}' to '{}'", prefix, m_Path.toString(), m_NewName); return MakeErrorResult(-6602, ss); } + if(validRename == 2) + { + if(m_Overwrite) + { + 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{-6603,ss}); + } + } + else + { + std::string ss = fmt::format("{}Another object exists with that name, will not rename DataObject at '{}' to '{}'", prefix, m_Path.toString(), m_NewName); + return MakeErrorResult(-6604, ss); + } + } dataObject->rename(m_NewName); return {}; @@ -51,4 +69,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..cbe20c7533 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 From faccc4fd011eabaac38c9860299af0a846db7715 Mon Sep 17 00:00:00 2001 From: nyoungbq Date: Fri, 12 Apr 2024 16:26:45 -0400 Subject: [PATCH 2/9] Update RenameDataObject.cpp, RenameDataObject.hpp, BaseGroup.cpp, and 4 more files --- .../SimplnxCore/Filters/RenameDataObject.cpp | 134 ++++++++++++++++++ .../SimplnxCore/Filters/RenameDataObject.hpp | 106 ++++++++++++++ src/simplnx/DataStructure/BaseGroup.cpp | 5 + src/simplnx/DataStructure/BaseGroup.hpp | 5 + src/simplnx/DataStructure/DataObject.cpp | 14 +- src/simplnx/DataStructure/DataObject.hpp | 2 +- .../Filter/Actions/RenameDataAction.cpp | 86 +++++++++-- 7 files changed, 333 insertions(+), 19 deletions(-) create mode 100644 src/Plugins/SimplnxCore/src/SimplnxCore/Filters/RenameDataObject.cpp create mode 100644 src/Plugins/SimplnxCore/src/SimplnxCore/Filters/RenameDataObject.hpp diff --git a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/RenameDataObject.cpp b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/RenameDataObject.cpp new file mode 100644 index 0000000000..38800f75c2 --- /dev/null +++ b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/RenameDataObject.cpp @@ -0,0 +1,134 @@ +#include "RenameDataObject.hpp" + +#include "simplnx/DataStructure/DataGroup.hpp" +#include "simplnx/Filter/Actions/RenameDataAction.hpp" +#include "simplnx/Parameters/DataPathSelectionParameter.hpp" +#include "simplnx/Parameters/StringParameter.hpp" +#include "simplnx/Utilities/DataArrayUtilities.hpp" + +#include "simplnx/Utilities/SIMPLConversion.hpp" + +#include + +using namespace nx::core; + +namespace +{ +constexpr int32 k_EmptyParameterError = -520; +} // namespace + +namespace nx::core +{ + +//------------------------------------------------------------------------------ +std::string RenameDataObject::name() const +{ + return FilterTraits::name; +} + +//------------------------------------------------------------------------------ +std::string RenameDataObject::className() const +{ + return FilterTraits::className; +} + +//------------------------------------------------------------------------------ +Uuid RenameDataObject::uuid() const +{ + return FilterTraits::uuid; +} + +//------------------------------------------------------------------------------ +std::string RenameDataObject::humanName() const +{ + return "Rename DataObject"; +} + +//------------------------------------------------------------------------------ +std::vector RenameDataObject::defaultTags() const +{ + return {className(), "Data Management", "Rename", "Data Structure", "Data Object"}; +} + +//------------------------------------------------------------------------------ +Parameters RenameDataObject::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; +} + +//------------------------------------------------------------------------------ +IFilter::UniquePointer RenameDataObject::clone() const +{ + return std::make_unique(); +} + +//------------------------------------------------------------------------------ +IFilter::PreflightResult RenameDataObject::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, allowOverwrite); + + OutputActions actions; + actions.appendAction(std::move(action)); + return {std::move(actions)}; +} + +//------------------------------------------------------------------------------ +Result<> RenameDataObject::executeImpl(DataStructure& data, const Arguments& args, const PipelineFilter* pipelineNode, const MessageHandler& messageHandler, const std::atomic_bool& shouldCancel) const +{ + return {}; +} + +namespace +{ +namespace SIMPL +{ +constexpr StringLiteral k_SelectedAttributeMatrixPathKey = "SelectedAttributeMatrixPath"; +constexpr StringLiteral k_NewAttributeMatrixKey = "NewAttributeMatrix"; +constexpr StringLiteral k_SelectedDataContainerNameKey = "SelectedDataContainerName"; +constexpr StringLiteral k_NewDataContainerNameKey = "NewDataContainerName"; +constexpr StringLiteral k_SelectedArrayPathKey = "SelectedArrayPath"; +constexpr StringLiteral k_NewArrayNameKey = "NewArrayName"; +} // namespace SIMPL +} // namespace + +Result RenameDataObject::FromSIMPLJson(const nlohmann::json& json) +{ + Arguments args = RenameDataObject().getDefaultArguments(); + + std::vector> results; + + if(json.contains(SIMPL::k_SelectedArrayPathKey.str())) + { + results.push_back(SIMPLConversion::ConvertParameter(args, json, SIMPL::k_SelectedArrayPathKey, k_SourceDataObjectPath_Key)); + results.push_back(SIMPLConversion::ConvertParameter(args, json, SIMPL::k_NewArrayNameKey, k_NewName_Key)); + } + else if(json.contains(SIMPL::k_SelectedAttributeMatrixPathKey)) + { + results.push_back( + SIMPLConversion::ConvertParameter(args, json, SIMPL::k_SelectedAttributeMatrixPathKey, k_SourceDataObjectPath_Key)); + results.push_back(SIMPLConversion::ConvertParameter(args, json, SIMPL::k_NewAttributeMatrixKey, k_NewName_Key)); + } + else + { + results.push_back( + SIMPLConversion::ConvertParameter(args, json, SIMPL::k_SelectedDataContainerNameKey, k_SourceDataObjectPath_Key)); + results.push_back(SIMPLConversion::ConvertParameter(args, json, SIMPL::k_NewDataContainerNameKey, k_NewName_Key)); + } + + Result<> conversionResult = MergeResults(std::move(results)); + + return ConvertResultTo(std::move(conversionResult), std::move(args)); +} +} // namespace nx::core diff --git a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/RenameDataObject.hpp b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/RenameDataObject.hpp new file mode 100644 index 0000000000..5958e8c46f --- /dev/null +++ b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/RenameDataObject.hpp @@ -0,0 +1,106 @@ +#pragma once + +#include "SimplnxCore/SimplnxCore_export.hpp" + +#include "simplnx/Common/StringLiteral.hpp" +#include "simplnx/Filter/FilterTraits.hpp" +#include "simplnx/Filter/IFilter.hpp" + +namespace nx::core +{ +/** + * @class RenameDataObject + * @brief RenameDataObject class is used to rename any DataObject. + */ +class SIMPLNXCORE_EXPORT RenameDataObject : public IFilter +{ +public: + RenameDataObject() = default; + ~RenameDataObject() noexcept override = default; + + RenameDataObject(const RenameDataObject&) = delete; + RenameDataObject(RenameDataObject&&) noexcept = delete; + + RenameDataObject& operator=(const RenameDataObject&) = delete; + RenameDataObject& operator=(RenameDataObject&&) 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"; + + /** + * @brief Reads SIMPL json and converts it simplnx Arguments. + * @param json + * @return Result + */ + static Result FromSIMPLJson(const nlohmann::json& json); + + /** + * @brief + * @return std::string + */ + std::string name() const override; + + /** + * @brief Returns the C++ classname of this filter. + * @return std::string + */ + std::string className() const override; + + /** + * @brief + * @return Uuid + */ + Uuid uuid() const override; + + /** + * @brief + * @return std::string + */ + std::string humanName() const override; + + /** + * @brief Returns the default tags for this filter. + * @return + */ + std::vector defaultTags() const override; + + /** + * @brief + * @return Parameters + */ + Parameters parameters() const override; + + /** + * @brief + * @return UniquePointer + */ + UniquePointer clone() const override; + +protected: + /** + * @brief + * @param data + * @param filterArgs + * @param messageHandler + * @param shouldCancel + * @return Result + */ + PreflightResult preflightImpl(const DataStructure& data, const Arguments& filterArgs, const MessageHandler& messageHandler, const std::atomic_bool& shouldCancel) const override; + + /** + * @brief + * @param dataStructure + * @param args + * @param pipelineNode + * @param messageHandler + * @param shouldCancel + * @return Result<> + */ + Result<> executeImpl(DataStructure& dataStructure, const Arguments& args, const PipelineFilter* pipelineNode, const MessageHandler& messageHandler, + const std::atomic_bool& shouldCancel) const override; +}; +} // namespace nx::core + +SIMPLNX_DEF_FILTER_TRAITS(nx::core, RenameDataObject, "911a3aa9-d3c2-4f66-9451-8861c4b726d5"); 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/DataStructure/DataObject.cpp b/src/simplnx/DataStructure/DataObject.cpp index 46a730a552..3d8823774f 100644 --- a/src/simplnx/DataStructure/DataObject.cpp +++ b/src/simplnx/DataStructure/DataObject.cpp @@ -162,38 +162,38 @@ std::string DataObject::getName() const return m_Name; } -int DataObject::canRename(const std::string& name) const +bool DataObject::canRename(const std::string& name) const { if(name == getName()) { - return 1; + return true; } if(!IsValidName(name)) { - return 0; + return false; } const auto* dataStructPtr = getDataStructure(); if(dataStructPtr == nullptr) { - return 0; + return false; } - return (std::any_of(m_ParentList.cbegin(), m_ParentList.cend(), [dataStructPtr, name](IdType parentId) { + return !std::any_of(m_ParentList.cbegin(), m_ParentList.cend(), [dataStructPtr, name](IdType parentId) { const auto* baseGroupPtr = dataStructPtr->getDataAs(parentId); if(baseGroupPtr == nullptr) { std::cout << "DataObject::canRename(name=" << name << ") cannot get baseGroup from parentId = " << parentId << std::endl; } return baseGroupPtr != nullptr && baseGroupPtr->contains(name); - })) ? 2 : 0; + }); } bool DataObject::rename(const std::string& name) { - if(canRename(name) != 1) + if(!canRename(name)) { return false; } diff --git a/src/simplnx/DataStructure/DataObject.hpp b/src/simplnx/DataStructure/DataObject.hpp index 387671713f..1198bde9ce 100644 --- a/src/simplnx/DataStructure/DataObject.hpp +++ b/src/simplnx/DataStructure/DataObject.hpp @@ -207,7 +207,7 @@ class SIMPLNX_EXPORT DataObject * @param name * @return int: 0 = false, 1 = true, 2 = duplicate object found */ - int canRename(const std::string& name) const; + bool canRename(const std::string& name) const; /** * @brief Attempts to rename the DataObject to the provided value. diff --git a/src/simplnx/Filter/Actions/RenameDataAction.cpp b/src/simplnx/Filter/Actions/RenameDataAction.cpp index 158ca7a94b..b7b293fb54 100644 --- a/src/simplnx/Filter/Actions/RenameDataAction.cpp +++ b/src/simplnx/Filter/Actions/RenameDataAction.cpp @@ -7,6 +7,37 @@ 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 childIds = dataStructure.getDataRefAs(id).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, bool overwrite) @@ -28,31 +59,64 @@ Result<> RenameDataAction::apply(DataStructure& dataStructure, Mode mode) const return MakeErrorResult(-6601, ss); } - int validRename = dataObject->canRename(m_NewName); - if(validRename == 0) - { - std::string ss = fmt::format("{}Could not rename DataObject at '{}' to '{}'", prefix, m_Path.toString(), m_NewName); - return MakeErrorResult(-6602, ss); - } - if(validRename == 2) + Result<> result = {}; + + bool validRename = dataObject->canRename(m_NewName); + + if(!validRename) { if(m_Overwrite) { if(mode == Mode::Preflight) { + std::vector pathVec = m_Path.getPathVector(); + + // The canRename() function returns 1 if the base object already has the objects new name + // so in that case we will never make it here + 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); + } + } + 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{-6603,ss}); + result.warnings().emplace_back(Warning{-6602, ss}); + } + else + { + DataObject::IdType targetId = std::numeric_limits::max(); + for(auto dataObjectID : dataStructure.getAllDataObjectIds()) + { + if(dataStructure.getData(dataObjectID)->getName() == m_NewName) + { + targetId = dataObjectID; + break; + } + } + + if(dataStructure.getDataAs(targetId) != nullptr) + { + // Recursive removal of overwritten object + result = MergeResults(result, ::TerminateNodesRecursively(dataStructure, targetId, mode, true)); + } + else + { + dataStructure.removeData(targetId); + } } } else { - std::string ss = fmt::format("{}Another object exists with that name, will not rename DataObject at '{}' to '{}'", prefix, m_Path.toString(), m_NewName); - return MakeErrorResult(-6604, ss); + std::string ss = fmt::format("{}Could not rename DataObject at '{}' to '{}'", prefix, m_Path.toString(), m_NewName); + return MakeErrorResult(-6603, ss); } } dataObject->rename(m_NewName); - return {}; + return result; } IDataAction::UniquePointer RenameDataAction::clone() const From 7d68392c677c1b271632ebb27a6c889e5ae63353 Mon Sep 17 00:00:00 2001 From: nyoungbq Date: Fri, 12 Apr 2024 17:21:59 -0400 Subject: [PATCH 3/9] Add tests --- .../SimplnxCore/test/RenameDataObjectTest.cpp | 83 +++++++++++++++++++ src/simplnx/Common/Result.hpp | 25 +++--- .../Filter/Actions/RenameDataAction.cpp | 2 +- 3 files changed, 95 insertions(+), 15 deletions(-) diff --git a/src/Plugins/SimplnxCore/test/RenameDataObjectTest.cpp b/src/Plugins/SimplnxCore/test/RenameDataObjectTest.cpp index 185492dcb8..48da13e5f0 100644 --- a/src/Plugins/SimplnxCore/test/RenameDataObjectTest.cpp +++ b/src/Plugins/SimplnxCore/test/RenameDataObjectTest.cpp @@ -66,3 +66,86 @@ 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}); + + RenameDataObject filter; + DataStructure dataStructure = UnitTest::CreateComplexMultiLevelDataGraph(); + Arguments args; + + args.insert(RenameDataObject::k_AllowOverwrite_Key, std::make_any(true)); + args.insert(RenameDataObject::k_NewName_Key, std::make_any(k_NewName)); + args.insert(RenameDataObject::k_DataObject_Key, std::make_any(k_DataPath)); + + auto preflightResult = filter.preflight(dataStructure, args); + SIMPLNX_RESULT_REQUIRE_VALID(preflightResult.outputActions); + + 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}); + + RenameDataObject filter; + DataStructure dataStructure = UnitTest::CreateComplexMultiLevelDataGraph(); + Arguments args; + + args.insert(RenameDataObject::k_AllowOverwrite_Key, std::make_any(true)); + args.insert(RenameDataObject::k_NewName_Key, std::make_any(k_NewName)); + args.insert(RenameDataObject::k_DataObject_Key, std::make_any(k_DataPath)); + + auto preflightResult = filter.preflight(dataStructure, args); + SIMPLNX_RESULT_REQUIRE_INVALID(preflightResult.outputActions); + + bool errorFound = false; + for(const auto& error : preflightResult.outputActions.errors()) + { + if(error.code == -6601) + { + errorFound = true; + } + } + REQUIRE(errorFound); + + auto result = filter.execute(dataStructure, args); + SIMPLNX_RESULT_REQUIRE_INVALID(result.result); +} diff --git a/src/simplnx/Common/Result.hpp b/src/simplnx/Common/Result.hpp index a2b15a7deb..5bb9b52f75 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()) { @@ -300,7 +287,17 @@ 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.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/Filter/Actions/RenameDataAction.cpp b/src/simplnx/Filter/Actions/RenameDataAction.cpp index b7b293fb54..2c08321902 100644 --- a/src/simplnx/Filter/Actions/RenameDataAction.cpp +++ b/src/simplnx/Filter/Actions/RenameDataAction.cpp @@ -71,7 +71,7 @@ Result<> RenameDataAction::apply(DataStructure& dataStructure, Mode mode) const { std::vector pathVec = m_Path.getPathVector(); - // The canRename() function returns 1 if the base object already has the objects new name + // The canRename() function returns true if the base object already has the objects new name // so in that case we will never make it here for(const auto& name : pathVec) { From e89172e372d38067f3000b78faa9307e3040e7af Mon Sep 17 00:00:00 2001 From: nyoungbq Date: Wed, 17 Apr 2024 06:52:19 -0400 Subject: [PATCH 4/9] share validation in the action --- .../Filter/Actions/RenameDataAction.cpp | 62 +++++++++---------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/src/simplnx/Filter/Actions/RenameDataAction.cpp b/src/simplnx/Filter/Actions/RenameDataAction.cpp index 2c08321902..df1e2b0c97 100644 --- a/src/simplnx/Filter/Actions/RenameDataAction.cpp +++ b/src/simplnx/Filter/Actions/RenameDataAction.cpp @@ -61,42 +61,42 @@ Result<> RenameDataAction::apply(DataStructure& dataStructure, Mode mode) const Result<> result = {}; - bool validRename = dataObject->canRename(m_NewName); - - if(!validRename) + if(m_Overwrite) { - if(m_Overwrite) + if(dataObject->getName() == m_NewName) { - if(mode == Mode::Preflight) + return result; + } + + std::vector pathVec = m_Path.getPathVector(); + for(const auto& name : pathVec) + { + if(name == m_NewName) { - std::vector pathVec = m_Path.getPathVector(); + 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); + } + } - // The canRename() function returns true if the base object already has the objects new name - // so in that case we will never make it here - 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 { - DataObject::IdType targetId = std::numeric_limits::max(); - for(auto dataObjectID : dataStructure.getAllDataObjectIds()) - { - if(dataStructure.getData(dataObjectID)->getName() == m_NewName) - { - targetId = dataObjectID; - break; - } - } - if(dataStructure.getDataAs(targetId) != nullptr) { // Recursive removal of overwritten object @@ -108,11 +108,11 @@ Result<> RenameDataAction::apply(DataStructure& dataStructure, Mode mode) const } } } - else - { - std::string ss = fmt::format("{}Could not rename DataObject at '{}' to '{}'", prefix, m_Path.toString(), m_NewName); - return MakeErrorResult(-6603, ss); - } + } + 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(-6603, ss); } dataObject->rename(m_NewName); From a8d4ef7aeef2e948f3ada668dd543c9dbc65a0be Mon Sep 17 00:00:00 2001 From: nyoungbq Date: Thu, 25 Apr 2024 16:46:34 -0400 Subject: [PATCH 5/9] post key rename patch --- src/Plugins/SimplnxCore/test/RenameDataObjectTest.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Plugins/SimplnxCore/test/RenameDataObjectTest.cpp b/src/Plugins/SimplnxCore/test/RenameDataObjectTest.cpp index 48da13e5f0..6d5d6d00c5 100644 --- a/src/Plugins/SimplnxCore/test/RenameDataObjectTest.cpp +++ b/src/Plugins/SimplnxCore/test/RenameDataObjectTest.cpp @@ -78,7 +78,7 @@ TEST_CASE("SimplnxCore::RenameDataAction(Valid Overwrite)", "[SimplnxCore][Renam args.insert(RenameDataObject::k_AllowOverwrite_Key, std::make_any(true)); args.insert(RenameDataObject::k_NewName_Key, std::make_any(k_NewName)); - args.insert(RenameDataObject::k_DataObject_Key, std::make_any(k_DataPath)); + args.insert(RenameDataObject::k_SourceDataObjectPath_Key, std::make_any(k_DataPath)); auto preflightResult = filter.preflight(dataStructure, args); SIMPLNX_RESULT_REQUIRE_VALID(preflightResult.outputActions); @@ -126,12 +126,13 @@ TEST_CASE("SimplnxCore::RenameDataAction(InValid Overwrite)", "[SimplnxCore][Ren static const DataPath k_DataPath({Constants::k_GroupAName, Constants::k_GroupCName, Constants::k_GroupDName, Constants::k_ArrayIName}); RenameDataObject filter; + DataStructure dataStructure = UnitTest::CreateComplexMultiLevelDataGraph(); Arguments args; args.insert(RenameDataObject::k_AllowOverwrite_Key, std::make_any(true)); args.insert(RenameDataObject::k_NewName_Key, std::make_any(k_NewName)); - args.insert(RenameDataObject::k_DataObject_Key, std::make_any(k_DataPath)); + args.insert(RenameDataObject::k_SourceDataObjectPath_Key, std::make_any(k_DataPath)); auto preflightResult = filter.preflight(dataStructure, args); SIMPLNX_RESULT_REQUIRE_INVALID(preflightResult.outputActions); From 16a705046a1a113fb6d33b76f31b908c8cc7371d Mon Sep 17 00:00:00 2001 From: nyoungbq Date: Tue, 30 Apr 2024 11:34:25 -0400 Subject: [PATCH 6/9] Result modifications, action bug fixes, and functional tests --- .../SimplnxCore/test/RenameDataObjectTest.cpp | 29 ++++++++++--------- src/simplnx/Common/Result.hpp | 2 +- .../Filter/Actions/RenameDataAction.cpp | 12 +++++--- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/src/Plugins/SimplnxCore/test/RenameDataObjectTest.cpp b/src/Plugins/SimplnxCore/test/RenameDataObjectTest.cpp index 6d5d6d00c5..a0283519b5 100644 --- a/src/Plugins/SimplnxCore/test/RenameDataObjectTest.cpp +++ b/src/Plugins/SimplnxCore/test/RenameDataObjectTest.cpp @@ -83,15 +83,16 @@ TEST_CASE("SimplnxCore::RenameDataAction(Valid Overwrite)", "[SimplnxCore][Renam auto preflightResult = filter.preflight(dataStructure, args); SIMPLNX_RESULT_REQUIRE_VALID(preflightResult.outputActions); - bool warningFound = false; - for(const auto& warning : preflightResult.outputActions.warnings()) - { - if(warning.code == -6602) - { - warningFound = true; - } - } - REQUIRE(warningFound); + // 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); @@ -135,10 +136,13 @@ TEST_CASE("SimplnxCore::RenameDataAction(InValid Overwrite)", "[SimplnxCore][Ren args.insert(RenameDataObject::k_SourceDataObjectPath_Key, std::make_any(k_DataPath)); auto preflightResult = filter.preflight(dataStructure, args); - SIMPLNX_RESULT_REQUIRE_INVALID(preflightResult.outputActions); + 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 : preflightResult.outputActions.errors()) + for(const auto& error : result.result.errors()) { if(error.code == -6601) { @@ -146,7 +150,4 @@ TEST_CASE("SimplnxCore::RenameDataAction(InValid Overwrite)", "[SimplnxCore][Ren } } REQUIRE(errorFound); - - auto result = filter.execute(dataStructure, args); - SIMPLNX_RESULT_REQUIRE_INVALID(result.result); } diff --git a/src/simplnx/Common/Result.hpp b/src/simplnx/Common/Result.hpp index 5bb9b52f75..280ec0f9c2 100644 --- a/src/simplnx/Common/Result.hpp +++ b/src/simplnx/Common/Result.hpp @@ -286,7 +286,7 @@ inline Result MergeResults(Result first, Result second) } } - Result result = errors.empty() ? Result{} : Result{nonstd::make_unexpected(std::move(errors))}; + 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()) diff --git a/src/simplnx/Filter/Actions/RenameDataAction.cpp b/src/simplnx/Filter/Actions/RenameDataAction.cpp index df1e2b0c97..e4828e368d 100644 --- a/src/simplnx/Filter/Actions/RenameDataAction.cpp +++ b/src/simplnx/Filter/Actions/RenameDataAction.cpp @@ -23,12 +23,16 @@ Result<> TerminateNodesRecursively(DataStructure& dataStructure, DataObject::IdT } } - auto childIds = dataStructure.getDataRefAs(id).GetChildrenIds(); - if(!childIds.empty()) + auto* baseGroup = dataStructure.getDataAs(id); + if(baseGroup != nullptr) { - for(const auto& childId : childIds) + auto childIds = baseGroup->GetChildrenIds(); + if(!childIds.empty()) { - result = MergeResults(result, std::move(TerminateNodesRecursively(dataStructure, childId, mode, checkDependence))); + for(const auto& childId : childIds) + { + result = MergeResults(result, std::move(TerminateNodesRecursively(dataStructure, childId, mode, checkDependence))); + } } } From 181c65be1f42fab1a86184f8a1e0e806521a1864 Mon Sep 17 00:00:00 2001 From: nyoungbq Date: Tue, 30 Apr 2024 11:54:05 -0400 Subject: [PATCH 7/9] post filter renaming patch --- .../SimplnxCore/Filters/RenameDataObject.cpp | 134 ------------------ .../SimplnxCore/Filters/RenameDataObject.hpp | 106 -------------- .../Filters/RenameDataObjectFilter.cpp | 5 +- .../Filters/RenameDataObjectFilter.hpp | 1 + .../SimplnxCore/test/RenameDataObjectTest.cpp | 16 +-- 5 files changed, 13 insertions(+), 249 deletions(-) delete mode 100644 src/Plugins/SimplnxCore/src/SimplnxCore/Filters/RenameDataObject.cpp delete mode 100644 src/Plugins/SimplnxCore/src/SimplnxCore/Filters/RenameDataObject.hpp diff --git a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/RenameDataObject.cpp b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/RenameDataObject.cpp deleted file mode 100644 index 38800f75c2..0000000000 --- a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/RenameDataObject.cpp +++ /dev/null @@ -1,134 +0,0 @@ -#include "RenameDataObject.hpp" - -#include "simplnx/DataStructure/DataGroup.hpp" -#include "simplnx/Filter/Actions/RenameDataAction.hpp" -#include "simplnx/Parameters/DataPathSelectionParameter.hpp" -#include "simplnx/Parameters/StringParameter.hpp" -#include "simplnx/Utilities/DataArrayUtilities.hpp" - -#include "simplnx/Utilities/SIMPLConversion.hpp" - -#include - -using namespace nx::core; - -namespace -{ -constexpr int32 k_EmptyParameterError = -520; -} // namespace - -namespace nx::core -{ - -//------------------------------------------------------------------------------ -std::string RenameDataObject::name() const -{ - return FilterTraits::name; -} - -//------------------------------------------------------------------------------ -std::string RenameDataObject::className() const -{ - return FilterTraits::className; -} - -//------------------------------------------------------------------------------ -Uuid RenameDataObject::uuid() const -{ - return FilterTraits::uuid; -} - -//------------------------------------------------------------------------------ -std::string RenameDataObject::humanName() const -{ - return "Rename DataObject"; -} - -//------------------------------------------------------------------------------ -std::vector RenameDataObject::defaultTags() const -{ - return {className(), "Data Management", "Rename", "Data Structure", "Data Object"}; -} - -//------------------------------------------------------------------------------ -Parameters RenameDataObject::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; -} - -//------------------------------------------------------------------------------ -IFilter::UniquePointer RenameDataObject::clone() const -{ - return std::make_unique(); -} - -//------------------------------------------------------------------------------ -IFilter::PreflightResult RenameDataObject::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, allowOverwrite); - - OutputActions actions; - actions.appendAction(std::move(action)); - return {std::move(actions)}; -} - -//------------------------------------------------------------------------------ -Result<> RenameDataObject::executeImpl(DataStructure& data, const Arguments& args, const PipelineFilter* pipelineNode, const MessageHandler& messageHandler, const std::atomic_bool& shouldCancel) const -{ - return {}; -} - -namespace -{ -namespace SIMPL -{ -constexpr StringLiteral k_SelectedAttributeMatrixPathKey = "SelectedAttributeMatrixPath"; -constexpr StringLiteral k_NewAttributeMatrixKey = "NewAttributeMatrix"; -constexpr StringLiteral k_SelectedDataContainerNameKey = "SelectedDataContainerName"; -constexpr StringLiteral k_NewDataContainerNameKey = "NewDataContainerName"; -constexpr StringLiteral k_SelectedArrayPathKey = "SelectedArrayPath"; -constexpr StringLiteral k_NewArrayNameKey = "NewArrayName"; -} // namespace SIMPL -} // namespace - -Result RenameDataObject::FromSIMPLJson(const nlohmann::json& json) -{ - Arguments args = RenameDataObject().getDefaultArguments(); - - std::vector> results; - - if(json.contains(SIMPL::k_SelectedArrayPathKey.str())) - { - results.push_back(SIMPLConversion::ConvertParameter(args, json, SIMPL::k_SelectedArrayPathKey, k_SourceDataObjectPath_Key)); - results.push_back(SIMPLConversion::ConvertParameter(args, json, SIMPL::k_NewArrayNameKey, k_NewName_Key)); - } - else if(json.contains(SIMPL::k_SelectedAttributeMatrixPathKey)) - { - results.push_back( - SIMPLConversion::ConvertParameter(args, json, SIMPL::k_SelectedAttributeMatrixPathKey, k_SourceDataObjectPath_Key)); - results.push_back(SIMPLConversion::ConvertParameter(args, json, SIMPL::k_NewAttributeMatrixKey, k_NewName_Key)); - } - else - { - results.push_back( - SIMPLConversion::ConvertParameter(args, json, SIMPL::k_SelectedDataContainerNameKey, k_SourceDataObjectPath_Key)); - results.push_back(SIMPLConversion::ConvertParameter(args, json, SIMPL::k_NewDataContainerNameKey, k_NewName_Key)); - } - - Result<> conversionResult = MergeResults(std::move(results)); - - return ConvertResultTo(std::move(conversionResult), std::move(args)); -} -} // namespace nx::core diff --git a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/RenameDataObject.hpp b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/RenameDataObject.hpp deleted file mode 100644 index 5958e8c46f..0000000000 --- a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/RenameDataObject.hpp +++ /dev/null @@ -1,106 +0,0 @@ -#pragma once - -#include "SimplnxCore/SimplnxCore_export.hpp" - -#include "simplnx/Common/StringLiteral.hpp" -#include "simplnx/Filter/FilterTraits.hpp" -#include "simplnx/Filter/IFilter.hpp" - -namespace nx::core -{ -/** - * @class RenameDataObject - * @brief RenameDataObject class is used to rename any DataObject. - */ -class SIMPLNXCORE_EXPORT RenameDataObject : public IFilter -{ -public: - RenameDataObject() = default; - ~RenameDataObject() noexcept override = default; - - RenameDataObject(const RenameDataObject&) = delete; - RenameDataObject(RenameDataObject&&) noexcept = delete; - - RenameDataObject& operator=(const RenameDataObject&) = delete; - RenameDataObject& operator=(RenameDataObject&&) 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"; - - /** - * @brief Reads SIMPL json and converts it simplnx Arguments. - * @param json - * @return Result - */ - static Result FromSIMPLJson(const nlohmann::json& json); - - /** - * @brief - * @return std::string - */ - std::string name() const override; - - /** - * @brief Returns the C++ classname of this filter. - * @return std::string - */ - std::string className() const override; - - /** - * @brief - * @return Uuid - */ - Uuid uuid() const override; - - /** - * @brief - * @return std::string - */ - std::string humanName() const override; - - /** - * @brief Returns the default tags for this filter. - * @return - */ - std::vector defaultTags() const override; - - /** - * @brief - * @return Parameters - */ - Parameters parameters() const override; - - /** - * @brief - * @return UniquePointer - */ - UniquePointer clone() const override; - -protected: - /** - * @brief - * @param data - * @param filterArgs - * @param messageHandler - * @param shouldCancel - * @return Result - */ - PreflightResult preflightImpl(const DataStructure& data, const Arguments& filterArgs, const MessageHandler& messageHandler, const std::atomic_bool& shouldCancel) const override; - - /** - * @brief - * @param dataStructure - * @param args - * @param pipelineNode - * @param messageHandler - * @param shouldCancel - * @return Result<> - */ - Result<> executeImpl(DataStructure& dataStructure, const Arguments& args, const PipelineFilter* pipelineNode, const MessageHandler& messageHandler, - const std::atomic_bool& shouldCancel) const override; -}; -} // namespace nx::core - -SIMPLNX_DEF_FILTER_TRAITS(nx::core, RenameDataObject, "911a3aa9-d3c2-4f66-9451-8861c4b726d5"); 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 a0283519b5..9ae87f46d9 100644 --- a/src/Plugins/SimplnxCore/test/RenameDataObjectTest.cpp +++ b/src/Plugins/SimplnxCore/test/RenameDataObjectTest.cpp @@ -72,13 +72,13 @@ TEST_CASE("SimplnxCore::RenameDataAction(Valid Overwrite)", "[SimplnxCore][Renam 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}); - RenameDataObject filter; + RenameDataObjectFilter filter; DataStructure dataStructure = UnitTest::CreateComplexMultiLevelDataGraph(); Arguments args; - args.insert(RenameDataObject::k_AllowOverwrite_Key, std::make_any(true)); - args.insert(RenameDataObject::k_NewName_Key, std::make_any(k_NewName)); - args.insert(RenameDataObject::k_SourceDataObjectPath_Key, std::make_any(k_DataPath)); + 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); @@ -126,14 +126,14 @@ TEST_CASE("SimplnxCore::RenameDataAction(InValid Overwrite)", "[SimplnxCore][Ren 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}); - RenameDataObject filter; + RenameDataObjectFilter filter; DataStructure dataStructure = UnitTest::CreateComplexMultiLevelDataGraph(); Arguments args; - args.insert(RenameDataObject::k_AllowOverwrite_Key, std::make_any(true)); - args.insert(RenameDataObject::k_NewName_Key, std::make_any(k_NewName)); - args.insert(RenameDataObject::k_SourceDataObjectPath_Key, std::make_any(k_DataPath)); + 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); From a2963f471366f9da21f7db2b6eb46e4ffb211401 Mon Sep 17 00:00:00 2001 From: nyoungbq Date: Tue, 30 Apr 2024 12:00:05 -0400 Subject: [PATCH 8/9] clang formatting --- src/simplnx/Filter/Actions/RenameDataAction.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/simplnx/Filter/Actions/RenameDataAction.hpp b/src/simplnx/Filter/Actions/RenameDataAction.hpp index cbe20c7533..e2e07083d2 100644 --- a/src/simplnx/Filter/Actions/RenameDataAction.hpp +++ b/src/simplnx/Filter/Actions/RenameDataAction.hpp @@ -52,7 +52,7 @@ class SIMPLNX_EXPORT RenameDataAction : public IDataAction * @brief Returns the overwrite value * @return */ - bool overwrite() const; + bool overwrite() const; private: std::string m_NewName; From 1d2fe16b97359f2f317d2bdf7a0e941dfdf480d0 Mon Sep 17 00:00:00 2001 From: Nathan Young Date: Wed, 1 May 2024 13:12:15 -0400 Subject: [PATCH 9/9] Apply suggestions from code review these were reverted changes that got missed after the renaming rebase Co-authored-by: Matthew Marine --- .../Filters/Algorithms/ITKImportFijiMontage.cpp | 2 +- src/simplnx/DataStructure/DataObject.hpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Plugins/ITKImageProcessing/src/ITKImageProcessing/Filters/Algorithms/ITKImportFijiMontage.cpp b/src/Plugins/ITKImageProcessing/src/ITKImageProcessing/Filters/Algorithms/ITKImportFijiMontage.cpp index 2b87ce2ec2..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()) != 1) + if(gray.canRename(imageDataPath.getTargetName()) == false) { return MakeErrorResult(-18543, fmt::format("Unable to rename the grayscale array to {}", imageDataPath.getTargetName())); } diff --git a/src/simplnx/DataStructure/DataObject.hpp b/src/simplnx/DataStructure/DataObject.hpp index 1198bde9ce..adc004913c 100644 --- a/src/simplnx/DataStructure/DataObject.hpp +++ b/src/simplnx/DataStructure/DataObject.hpp @@ -205,7 +205,7 @@ class SIMPLNX_EXPORT DataObject * @brief Checks and returns if the DataObject can be renamed to the provided * value. * @param name - * @return int: 0 = false, 1 = true, 2 = duplicate object found + * @return bool */ bool canRename(const std::string& name) const;