Skip to content

Robustify links reading #5995

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.

Sign up for GitHub

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

Draft
alxbilger wants to merge 3 commits into
base: master
Choose a base branch
from
+734 −208
Draft

Robustify links reading #5995

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fc88369
Robustify links reading
alxbilger Mar 6, 2026
d7f651e
remove wrong ".." logic
alxbilger Mar 9, 2026
b55892e
fix @./ (#11)
Lucas-TJ Mar 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 33 additions & 23 deletions Sofa/framework/Core/src/sofa/core/objectmodel/BaseLink.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ bool BaseLink::ParseString(const std::string& text, std::string* path, std::stri
{
if (owner)
{
msg_error(owner) << "ERROR parsing Link \""<<text<<"\": first character should be '@'.";
msg_error(owner) << "Parsing Link \""<<text<<"\": first character should be '@'.";
}
else
{
Expand Down Expand Up @@ -274,8 +274,25 @@ bool BaseLink::read( const std::string& str )
if (owner == nullptr)
return false;

std::istringstream istr(str);
std::string path;
auto cleanedPath = sofa::helper::removeLeadingCharacter(str, ' ');

auto first = cleanedPath.find('@');
if (first != std::string::npos)
{
if (first != 0)
{
//the first characters don't start with '@'. They will be omitted.
msg_error(owner) << "Parsing Link \"" << str <<"\": first character should be '@'. All characters before the first '@' will be omitted.";
cleanedPath = cleanedPath.substr(first);
}
}
else
{
//the string does not contain any '@'
msg_error(owner) << "Parsing Link \"" << str << "\": no '@' character found. A valid path must start with a '@'";
return false;
}


/// Find the target of each path, and stores each targets in
/// a temporary vector of (pointer, path) pairs.
Expand All @@ -284,26 +301,19 @@ bool BaseLink::read( const std::string& str )
bool ok = true;
std::vector< std::pair<Base*, std::string> > entries;

/// Cut the path using space as a separator. This has several
/// questionnable consequence among which space are not allowed in part of a path (so no name containing space)
/// tokenizing the path using '@' as a separator would solve the issue.
while (istr >> path)
auto paths = sofa::helper::split(std::string(cleanedPath), '@');
std::erase_if(paths, [](const std::string& path) { return path.empty(); });
std::transform(paths.begin(), paths.end(), paths.begin(), [](const std::string& path) { return "@" + path; });

for (const auto& path : paths)
{
if (path[0] != '@')
{
msg_error(owner) << "Parsing Link \"" << path <<"\": first character should be '@'.";
ok = false;
}
else
{
/// Check if the path is pointing to any object of Base type.
Base* ptr = PathResolver::FindBaseFromClassAndPath(owner, getDestClass(), path);
/// Check if the path is pointing to any object of Base type.
Base* ptr = PathResolver::FindBaseFromClassAndPath(owner, getDestClass(), path);

/// We found either a valid Base object or none.
/// ptr can be nullptr, as the destination can be added later in the graph
/// instead, we will check for failed links after init is completed
entries.emplace_back(ptr, path);
}
/// We found either a valid Base object or none.
/// ptr can be nullptr, as the destination can be added later in the graph
/// instead, we will check for failed links after init is completed
entries.emplace_back(ptr, path);
}

/// Check for the case where multiple link has been read while we are a single multilink.
Expand All @@ -316,9 +326,9 @@ bool BaseLink::read( const std::string& str )

/// Add the detected objects that are not already present to the container of this Link
clear();
for (const auto& [base, path] : entries)
for (const auto& [base2, path2] : entries)
{
ok = add(base, path) && ok;
ok = add(base2, path2) && ok;
}
return ok;
}
Expand Down
222 changes: 221 additions & 1 deletion Sofa/framework/Core/test/objectmodel/SingleLink_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,11 @@
* Contact information: contact@sofa-framework.org *
******************************************************************************/
#include <sofa/core/objectmodel/BaseObject.h>
#include <sofa/simulation/Node.h>
#include <sofa/simulation/Simulation.h>
#include <sofa/simulation/fwd.h>
using sofa::core::objectmodel::BaseObject ;
#include <sofa/core/objectmodel/BaseNode.h>
#include <sofa/simulation/Node.h>
using sofa::core::objectmodel::BaseNode ;

#include <sofa/core/objectmodel/Link.h>
Expand All @@ -45,6 +48,7 @@ class SingleLink_test: public BaseTest
SingleLink<sofa::core::objectmodel::BaseObject, sofa::core::objectmodel::BaseObject, BaseLink::FLAG_DOUBLELINK|BaseLink::FLAG_STRONGLINK|BaseLink::FLAG_STOREPATH > m_link ;
sofa::core::objectmodel::BaseObject::SPtr m_dst ;
sofa::core::objectmodel::BaseObject::SPtr m_src ;
sofa::simulation::Node::SPtr m_root { nullptr };

/// Create a link to an object.
void doSetUp() override
Expand All @@ -57,6 +61,20 @@ class SingleLink_test: public BaseTest
m_src->addLink(&m_link);
m_link.add(m_dst.get());
}

void setupContext()
{
m_root = sofa::simulation::getSimulation()->createNewGraph("root");

m_root->addObject(this->m_src);
m_root->addObject(this->m_dst);

m_link.set(nullptr);
ASSERT_EQ(m_link.get(), nullptr);

m_src->addLink(&m_link);
m_link.setOwner(m_src.get());
}
};

TEST_F(SingleLink_test, checkAccess )
Expand Down Expand Up @@ -142,3 +160,205 @@ TEST_F(SingleLink_test, checkEmptyness )
m_link.clear();
ASSERT_EQ( m_link.size(), 0 );
}

TEST_F(SingleLink_test, readWithoutContext)
{
m_link.set(nullptr);
ASSERT_EQ(m_link.get(), nullptr);

const bool success = m_link.read("@/destination");
ASSERT_FALSE(success);
ASSERT_EQ(m_link.get(), nullptr);
}



TEST_F(SingleLink_test, readWithContextEmptyString)
{
setupContext();

const bool success = m_link.read("");
ASSERT_TRUE(success);
ASSERT_EQ(m_link.get(), nullptr);
}

TEST_F(SingleLink_test, readWithContextJustAArobase)
{
setupContext();

// the path points to an invalid object, but it's ok
const bool success = m_link.read("@");
ASSERT_TRUE(success);
ASSERT_EQ(m_link.get(), nullptr);
}

TEST_F(SingleLink_test, readWithContextValidLink)
{
setupContext();

const bool success = m_link.read("@/destination");
ASSERT_TRUE(success);
ASSERT_EQ(m_link.get(), m_dst.get());
}

TEST_F(SingleLink_test, readWithContextInvalidLink)
{
setupContext();

// the path points to an invalid object, but it's ok
const bool success = m_link.read("@/foo");
ASSERT_TRUE(success);
ASSERT_EQ(m_link.get(), nullptr);
}

TEST_F(SingleLink_test, readWithContextValidLinkDoubleSlash)
{
setupContext();

const bool success = m_link.read("@//destination");
ASSERT_TRUE(success);
ASSERT_EQ(m_link.get(), m_dst.get());
}

TEST_F(SingleLink_test, readWithContextValidLinkNoSlash)
{
setupContext();

const bool success = m_link.read("@destination");
ASSERT_TRUE(success);
ASSERT_EQ(m_link.get(), m_dst.get());
}

TEST_F(SingleLink_test, readWithContextValidLinkLeadingSpaces)
{
setupContext();

const bool success = m_link.read(" @/destination");
ASSERT_TRUE(success);
ASSERT_EQ(m_link.get(), m_dst.get());
ASSERT_EQ(m_link.getSize(), 1);
}

TEST_F(SingleLink_test, readWithContextValidLinkTrailingSpaces)
{
setupContext();

const bool success = m_link.read("@/destination ");
ASSERT_TRUE(success);
ASSERT_EQ(m_link.get(), m_dst.get());
ASSERT_EQ(m_link.getSize(), 1);
}

TEST_F(SingleLink_test, readWithContextValidLinkTrailingSpacesButTheyArePartOfThePath)
{
setupContext();

m_dst->setName("destination ");

const bool success = m_link.read("@/destination ");
ASSERT_TRUE(success);
ASSERT_EQ(m_link.get(), m_dst.get());
ASSERT_EQ(m_link.getSize(), 1);
}

TEST_F(SingleLink_test, readWithContextWithDot)
{
setupContext();
m_dst->setName("Component.With.Dots");

const bool success = m_link.read("@Component.With.Dots");
ASSERT_TRUE(success);
}

TEST_F(SingleLink_test, readWithContextWithSpace)
{
setupContext();
m_dst->setName("Backward Euler ODE Solver");

const bool success = m_link.read("@Backward Euler ODE Solver");
ASSERT_TRUE(success);
}

TEST_F(SingleLink_test, readComplex)
{
m_root = sofa::simulation::getSimulation()->createNewGraph("root");
m_root->addObject(this->m_src);

auto child1 = m_root->createChild("child 1");
auto child2 = child1->createChild("child 2");
child2->addObject(this->m_dst);
m_dst->setName("My Object");

m_link.set(nullptr);
ASSERT_EQ(m_link.get(), nullptr);

m_src->addLink(&m_link);
m_link.setOwner(m_src.get());

{
const bool success = m_link.read("@/child 1/child 2/My Object");
ASSERT_TRUE(success);
ASSERT_EQ(m_link.get(), m_dst.get());
ASSERT_EQ(m_link.getSize(), 1);
}

{
const bool success = m_link.read("@/child 2/My Object");
ASSERT_TRUE(success);
ASSERT_EQ(m_link.get(), nullptr);
ASSERT_EQ(m_link.getSize(), 1);
}

{
const bool success = m_link.read("@/child 1/child 2/../child 2/My Object");
ASSERT_TRUE(success);
ASSERT_EQ(m_link.get(), m_dst.get());
ASSERT_EQ(m_link.getSize(), 1);
}
}

TEST_F(SingleLink_test, readComplex2)
{
m_root = sofa::simulation::getSimulation()->createNewGraph("root");

auto child1 = m_root->createChild("child 1");
child1->addObject(this->m_src);

auto child2 = child1->createChild("child 2");
child2->addObject(this->m_dst);
m_dst->setName("My Object");

m_link.set(nullptr);
ASSERT_EQ(m_link.get(), nullptr);

m_src->addLink(&m_link);
m_link.setOwner(m_src.get());

{
const bool success = m_link.read("@/child 1/child 2/My Object");
ASSERT_TRUE(success);
ASSERT_EQ(m_link.get(), m_dst.get());
ASSERT_EQ(m_link.getSize(), 1);
}

{
const bool success = m_link.read("@child 2/My Object");
ASSERT_TRUE(success);
ASSERT_EQ(m_link.get(), m_dst.get());
ASSERT_EQ(m_link.getSize(), 1);
}

{
const bool success = m_link.read("@child 2/My Object "); //trailing spaces
ASSERT_TRUE(success);
ASSERT_EQ(m_link.get(), m_dst.get());
ASSERT_EQ(m_link.getSize(), 1);
}

{
const bool success = m_link.read("@/../child 1/child 2/My Object"); //ill-formed
ASSERT_TRUE(success);
ASSERT_EQ(m_link.get(), nullptr);
ASSERT_EQ(m_link.getSize(), 1);
}
}
2 changes: 2 additions & 0 deletions Sofa/framework/Helper/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ set(HEADER_FILES
${SRC_ROOT}/GenerateRigid.h
${SRC_ROOT}/IndexOpenMP.h
${SRC_ROOT}/LCPcalc.h
${SRC_ROOT}/LinkParser.h
${SRC_ROOT}/MarchingCubeUtility.h
${SRC_ROOT}/MatEigen.h
${SRC_ROOT}/MemoryManager.h
Expand Down Expand Up @@ -149,6 +150,7 @@ set(SOURCE_FILES
${SRC_ROOT}/Factory.cpp
${SRC_ROOT}/GenerateRigid.cpp
${SRC_ROOT}/LCPcalc.cpp
${SRC_ROOT}/LinkParser.cpp
${SRC_ROOT}/MarchingCubeUtility.cpp
${SRC_ROOT}/NameDecoder.cpp
${SRC_ROOT}/OptionsGroup.cpp
Expand Down
Loading
Loading