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

ObjectPath XML errors feedback #2341

Merged
merged 27 commits into from
Apr 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
593046d
No more crash when PackCollection::objectPath is wrong
MelReyCG Mar 15, 2023
cb89711
- Fixing getPath() so it starts with "/" instead of "m/"
MelReyCG Mar 15, 2023
b734b49
Made group searching errors clearer.
MelReyCG Mar 15, 2023
8d95760
- Improved FieldSpecification::objectPath errors clarity
MelReyCG Mar 15, 2023
cae0f90
Added a test for Group path methods and data hierarchy
MelReyCG Mar 16, 2023
a1f322e
indentation fix
MelReyCG Mar 16, 2023
2eff3e3
testMeshObjectPath now expect exceptions instead of errors
MelReyCG Mar 16, 2023
5528732
Merge remote-tracking branch 'origin/develop' into bugfix/rey/2320-xm…
MelReyCG Mar 16, 2023
f737cca
Added some doxygen throwing documentation
MelReyCG Mar 17, 2023
5ba1d88
fix : forgot to use a reference
MelReyCG Mar 17, 2023
ed942e3
Merge remote-tracking branch 'origin/develop' into bugfix/rey/2320-xm…
MelReyCG Mar 17, 2023
f165806
getPath() call on Problem node now returns "/"
MelReyCG Mar 23, 2023
a67ae55
shorter messages
MelReyCG Mar 23, 2023
9430d8f
code style & reformulations
MelReyCG Mar 28, 2023
74f3c18
Docs rewording
MelReyCG Mar 28, 2023
52309be
- modified messages after user feedback.
MelReyCG Mar 29, 2023
d0bdbbb
- code style
MelReyCG Mar 31, 2023
1912e66
variable naming
MelReyCG Apr 3, 2023
930dd64
updating "invalid MeshBody" error message.
MelReyCG Apr 3, 2023
aa9a930
Test to check the exception of getGroupByPath()
MelReyCG Apr 3, 2023
026b433
code style
MelReyCG Apr 3, 2023
7fb5afb
- Added a stringutilities::cstrlen (= compile-time strlen)
MelReyCG Apr 5, 2023
3fc7414
fixing a wrong namespace
MelReyCG Apr 6, 2023
97fd7e0
added c++17 comment
MelReyCG Apr 6, 2023
70abf6c
Code factorisation (THROW_IF)
MelReyCG Apr 13, 2023
23213e7
Merge branch 'develop' into bugfix/rey/2320-xml-errors-feedback
MelReyCG Apr 19, 2023
f9f3c73
GEOSX -> GEOS
MelReyCG Apr 19, 2023
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
19 changes: 19 additions & 0 deletions src/coreComponents/codingUtilities/StringUtilities.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,25 @@ array1d< T > fromStringToArray( string const & str )
template< typename T >
string toMetricPrefixString( T const & value );

/**
* @brief Compute the length of a constant string at compile-time.
*/
// TODO c++17: this function is to remove in favor of std::string_view
constexpr size_t cstrlen( char const * const str )
{
if( str )
{
char const * ptr = str;
for(; *ptr != '\0'; ++ptr )
{}
return ptr - str;
}
else
{
return 0;
}
}

} // namespace stringutilities
} // namespace geos

Expand Down
32 changes: 32 additions & 0 deletions src/coreComponents/common/Logger.cpp
MelReyCG marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,42 @@
// Source includes
#include "Logger.hpp"
#include "Path.hpp"
#include "codingUtilities/StringUtilities.hpp"

namespace geos
{

/**
* @brief Insert an exception message in another one.
* @param originalMsg original exception message (i.e. thrown from LVARRAY_THROW or GEOSX_THROW)
* @param msgToInsert message to insert at the top of the originalMsg
*/
std::string InsertExMsg( std::string const & originalMsg, std::string const & msgToInsert )
{
std::string newMsg( originalMsg );

size_t insertPos = 0;
// for readability purposes, we try to insert the message after the "***** Rank N: " or after "***** " instead of at the top.
static auto constexpr rankLogStart = "***** Rank ";
static auto constexpr rankLogEnd = ": ";
static auto constexpr simpleLogStart = "***** ";
if( ( insertPos = newMsg.find( rankLogStart ) ) != std::string::npos )
{
insertPos = newMsg.find( rankLogEnd, insertPos + stringutilities::cstrlen( rankLogStart ) )
+ stringutilities::cstrlen( rankLogEnd );
}
else if( ( insertPos = newMsg.find_last_of( simpleLogStart ) ) != std::string::npos )
{
insertPos += stringutilities::cstrlen( simpleLogStart );
}
newMsg.insert( insertPos, msgToInsert );
return newMsg;
}

InputError::InputError( std::exception const & subException, std::string const & msgToInsert ):
std::runtime_error( InsertExMsg( subException.what(), msgToInsert ) )
{}

namespace logger
{

Expand Down
7 changes: 7 additions & 0 deletions src/coreComponents/common/Logger.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,13 @@ struct InputError : public std::runtime_error
InputError( char const * const what ):
std::runtime_error( what )
{}

/**
* @brief Construct an InputError from an underlying exception.
* @param subException An exception to base this new one on.
* @param msgToInsert The error message. It will be inserted into the one inside of subException.
*/
InputError( std::exception const & subException, std::string const & msgToInsert );
};

/**
Expand Down
50 changes: 47 additions & 3 deletions src/coreComponents/dataRepository/Group.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ string Group::getPath() const
{
// In the Conduit node heirarchy everything begins with 'Problem', we should change it so that
// the ProblemManager actually uses the root Conduit Node but that will require a full rebaseline.
string const noProblem = getConduitNode().path().substr( std::strlen( dataRepository::keys::ProblemManager ) - 1 );
string const noProblem = getConduitNode().path().substr( stringutilities::cstrlen( dataRepository::keys::ProblemManager ) );
return noProblem.empty() ? "/" : noProblem;
}

Expand Down Expand Up @@ -267,6 +267,32 @@ string Group::dumpInputOptions() const
return rval;
}

string Group::dumpSubGroupsNames() const
{
if( numSubGroups() == 0 )
{
return getName() + " has no children.";
}
else
{
return "The children of " + getName() + " are: " +
"{ " + stringutilities::join( getSubGroupsNames(), ", " ) + " }";
}
}

string Group::dumpWrappersNames() const
{
if( numWrappers() == 0 )
{
return getName() + " has no wrappers.";
}
else
{
return "The wrappers of " + getName() + " are: " +
"{ " + stringutilities::join( getWrappersNames(), ", " ) + " }";
}
}

void Group::deregisterGroup( string const & name )
{
GEOS_ERROR_IF( !hasGroup( name ), "Group " << name << " doesn't exist." );
Expand Down Expand Up @@ -610,8 +636,10 @@ Group const & Group::getBaseGroupByPath( string const & path ) const
break;
}
}
GEOS_ERROR_IF( !foundTarget,
"Could not find the specified path from the starting group." );
GEOS_THROW_IF( !foundTarget,
"Could not find the specified path start.\n"<<
"Specified path is " << path,
std::domain_error );
}

string::size_type currentPosition;
Expand Down Expand Up @@ -650,5 +678,21 @@ PyTypeObject * Group::getPythonType() const
{ return geos::python::getPyGroupType(); }
#endif

std::vector< string > Group::getSubGroupsNames() const
{
std::vector< string > childrenNames;
childrenNames.reserve( numSubGroups() );
forSubGroups( [&]( Group const & subGroup ){ childrenNames.push_back( subGroup.getName() ); } );
return childrenNames;
}

std::vector< string > Group::getWrappersNames() const
{
std::vector< string > wrappersNames;
wrappersNames.reserve( numWrappers() );
forWrappers( [&]( WrapperBase const & wrapper ){ wrappersNames.push_back( wrapper.getName() ); } );
return wrappersNames;
}

} /* end namespace dataRepository */
} /* end namespace geos */
45 changes: 41 additions & 4 deletions src/coreComponents/dataRepository/Group.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,16 @@ class Group
*/
string dumpInputOptions() const;

/**
* @brief @return a comma separated string containing all sub groups name.
*/
string dumpSubGroupsNames() const;

/**
* @brief @return a comma separated string containing all wrappers name.
*/
string dumpWrappersNames() const;
untereiner marked this conversation as resolved.
Show resolved Hide resolved

///@}

//START_SPHINX_INCLUDE_REGISTER_GROUP
Expand Down Expand Up @@ -323,7 +333,11 @@ class Group
T & getGroup( KEY const & key )
{
Group * const child = m_subGroups[ key ];
GEOS_THROW_IF( child == nullptr, "Group " << getPath() << " doesn't have a child " << key, std::domain_error );
GEOS_THROW_IF( child == nullptr,
"Group " << getPath() << " has no child named " << key << std::endl
<< dumpSubGroupsNames(),
std::domain_error );

return dynamicCast< T & >( *child );
}

Expand All @@ -334,7 +348,11 @@ class Group
T const & getGroup( KEY const & key ) const
{
Group const * const child = m_subGroups[ key ];
GEOS_THROW_IF( child == nullptr, "Group " << getPath() << " doesn't have a child " << key, std::domain_error );
GEOS_THROW_IF( child == nullptr,
"Group " << getPath() << " has no child named " << key << std::endl
<< dumpSubGroupsNames(),
std::domain_error );

return dynamicCast< T const & >( *child );
}

Expand Down Expand Up @@ -380,6 +398,11 @@ class Group
*/
localIndex numSubGroups() const { return m_subGroups.size(); }

/**
* @return An array containing all sub groups keys
*/
std::vector< string > getSubGroupsNames() const;

/**
* @brief Check whether a sub-group exists.
* @param name the name of sub-group to search for
Expand Down Expand Up @@ -1047,7 +1070,11 @@ class Group
WrapperBase const & getWrapperBase( KEY const & key ) const
{
WrapperBase const * const wrapper = m_wrappers[ key ];
GEOS_THROW_IF( wrapper == nullptr, "Group " << getPath() << " doesn't have a child " << key, std::domain_error );
GEOS_THROW_IF( wrapper == nullptr,
"Group " << getPath() << " has no wrapper named " << key << std::endl
<< dumpWrappersNames(),
std::domain_error );

return *wrapper;
}

Expand All @@ -1058,7 +1085,11 @@ class Group
WrapperBase & getWrapperBase( KEY const & key )
{
WrapperBase * const wrapper = m_wrappers[ key ];
GEOS_THROW_IF( wrapper == nullptr, "Group " << getPath() << " doesn't have a child " << key, std::domain_error );
GEOS_THROW_IF( wrapper == nullptr,
"Group " << getPath() << " has no wrapper named " << key << std::endl
<< dumpWrappersNames(),
std::domain_error );

return *wrapper;
}

Expand Down Expand Up @@ -1090,6 +1121,11 @@ class Group
indexType numWrappers() const
{ return m_wrappers.size(); }

/**
* @return An array containing all wrappers keys
*/
std::vector< string > getWrappersNames() const;

///@}

/**
Expand Down Expand Up @@ -1244,6 +1280,7 @@ class Group

/**
* @brief Return the path of this Group in the data repository.
* Starts with '/' followed by the hierarchy of the children of the "Problem" in which the Group is.
* @return The path of this group in the data repository.
*/
string getPath() const;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,14 @@ FieldSpecificationBase::getCatalog()

void FieldSpecificationBase::setMeshObjectPath( Group const & meshBodies )
{
m_meshObjectPaths = std::make_unique< MeshObjectPath >( m_objectPath, meshBodies );
try
{
m_meshObjectPaths = std::make_unique< MeshObjectPath >( m_objectPath, meshBodies );
}
catch( InputError const & e )
{
throw InputError( e, getName() + " has a wrong objectPath: " + m_objectPath + "\n" );
}
}


Expand Down
Loading