Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH: RenameAction and Filter Add Overwrite Option #912

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ class IOHandler
// rename grayscale array to reflect original
{
auto& gray = m_DataStructure.getDataRefAs<IDataArray>(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()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ Parameters RenameDataObjectFilter::parameters() const
Parameters params;

params.insertSeparator(Parameters::Separator{"Input Parameters"});
params.insert(std::make_unique<BoolParameter>(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<DataPathSelectionParameter>(k_SourceDataObjectPath_Key, "DataObject to Rename", "DataPath pointing to the target DataObject", DataPath()));
params.insert(std::make_unique<StringParameter>(k_NewName_Key, "New Name", "Target DataObject name", ""));
return params;
Expand All @@ -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<bool>(k_AllowOverwrite_Key);
auto dataGroupPath = filterArgs.value<DataPath>(k_SourceDataObjectPath_Key);
auto newName = filterArgs.value<std::string>(k_NewName_Key);

auto action = std::make_unique<RenameDataAction>(dataGroupPath, newName);
auto action = std::make_unique<RenameDataAction>(dataGroupPath, newName, allowOverwrite);

OutputActions actions;
actions.appendAction(std::move(action));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down
85 changes: 85 additions & 0 deletions src/Plugins/SimplnxCore/test/RenameDataObjectTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool>(true));
args.insert(RenameDataObjectFilter::k_NewName_Key, std::make_any<std::string>(k_NewName));
args.insert(RenameDataObjectFilter::k_SourceDataObjectPath_Key, std::make_any<DataPath>(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<bool>(true));
args.insert(RenameDataObjectFilter::k_NewName_Key, std::make_any<std::string>(k_NewName));
args.insert(RenameDataObjectFilter::k_SourceDataObjectPath_Key, std::make_any<DataPath>(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);
}
27 changes: 12 additions & 15 deletions src/simplnx/Common/Result.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -259,19 +259,6 @@ inline bool ExtractResult(Result<> result, std::vector<Error>& errors, std::vect
template <typename T = void>
inline Result<T> MergeResults(Result<T> first, Result<T> second)
{
usize warningsSize = first.warnings().size() + second.warnings().size();
std::vector<Warning> 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())
{
Expand Down Expand Up @@ -299,8 +286,18 @@ inline Result<T> MergeResults(Result<T> first, Result<T> second)
}
}

Result<T> result = errors.empty() ? Result<T>{} : Result<T>{nonstd::make_unexpected(std::move(errors))};
result.warnings() = std::move(warnings);
Result<T> result = errors.empty() ? Result<T>{} : Result<T>{{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;
}

Expand Down
5 changes: 5 additions & 0 deletions src/simplnx/DataStructure/BaseGroup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -246,3 +246,8 @@ void BaseGroup::checkUpdatedIdsImpl(const std::vector<std::pair<IdType, IdType>>
{
m_DataMap.updateIds(updatedIds);
}

std::vector<DataObject::IdType> BaseGroup::GetChildrenIds()
{
return m_DataMap.getKeys();
}
5 changes: 5 additions & 0 deletions src/simplnx/DataStructure/BaseGroup.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,11 @@ class SIMPLNX_EXPORT BaseGroup : public DataObject
*/
std::vector<std::string> GetChildrenNames();

/**
* @brief Querys the DataMap for the object ids in m_DataMap
*/
std::vector<DataObject::IdType> GetChildrenIds();

protected:
/**
* @brief Creates a BaseGroup with the target DataStructure and name.
Expand Down
100 changes: 96 additions & 4 deletions src/simplnx/Filter/Actions/RenameDataAction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<BaseGroup>(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)
{
}

Expand All @@ -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<std::string> 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<DataObject::IdType>::max();
for(auto dataObjectID : dataStructure.getAllDataObjectIds())
{
if(dataStructure.getData(dataObjectID)->getName() == m_NewName)
{
targetId = dataObjectID;
break;
}
}

if(targetId != std::numeric_limits<DataObject::IdType>::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<BaseGroup>(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
Expand All @@ -51,4 +137,10 @@ const DataPath& RenameDataAction::path() const
{
return m_Path;
}

bool RenameDataAction::overwrite() const
{
return m_Overwrite;
}

} // namespace nx::core
9 changes: 8 additions & 1 deletion src/simplnx/Filter/Actions/RenameDataAction.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Loading