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.

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

XML string array errors feedback #2357

Merged
merged 10 commits into from
Jun 2, 2023
Merged

XML string array errors feedback #2357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
7601706
Adding context to errors when reading wrapper from xml
MelReyCG Mar 23, 2023
622f8d6
added XML hierarchy to the wrapper parsing error
MelReyCG Mar 24, 2023
a5fb1fe
Modified testXmlWrapper so it expect exceptions
MelReyCG Mar 27, 2023
4524704
Changed the regexs so the schema detects more errors
MelReyCG Mar 27, 2023
c36a052
updating LvArray submodule
MelReyCG Mar 27, 2023
081939c
Adding documentation
MelReyCG Mar 28, 2023
a123a15
added C++17 string_view comments
MelReyCG Apr 6, 2023
82e4ad0
Merge branch 'develop' into bugfix/rey/2320-xml-errors-feedback-3
MelReyCG Apr 27, 2023
2be7f24
schema update
MelReyCG Apr 27, 2023
c345791
Merge branch 'develop' into bugfix/rey/2320-xml-errors-feedback-3
wrtobin Jun 2, 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
30 changes: 24 additions & 6 deletions src/coreComponents/common/DataTypes.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -629,9 +629,11 @@ class rtTypes

// Define the component regexes:
// Regex to match an unsigned int (123, etc.)
// TODO c++17: Move to static constexpr std::string_view
string ru = "[\\d]+";

// Regex to match an signed int (-123, 455, +789, etc.)
// TODO c++17: Move to static constexpr std::string_view
string ri = "[+-]?[\\d]+";

// Regex to match a float (1, +2.3, -.4, 5.6e7, 8E-9, etc.)
Expand All @@ -641,15 +643,31 @@ class rtTypes
// [\\d]* matches any number of numbers following the decimal
// ([eE][-+]?[\\d]+|\\s*) matches an optional scientific notation number
// Note: the xsd regex implementation does not allow an empty branch, so use allow whitespace at the end
// TODO c++17: Move to static constexpr std::string_view
string rr = "[+-]?[\\d]*([\\d]\\.?|\\.[\\d])[\\d]*([eE][-+]?[\\d]+|\\s*)";

// Regex to match a string that does not contain the characters ,{}
string rs = "[^,\\{\\}]*";
// Regex to match a string that can't be empty and does not contain any whitespaces nor the characters ,{}
// TODO c++17: Move to static constexpr std::string_view
string rs = "[^,\\{\\}\\s]+\\s*";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need to take action with potential initial spaces (like toto) or is it OK?

Copy link
Contributor

@cssherman cssherman May 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pattern will be permissive of spaces at the end of the string, but not at the beginning, which should be fine, right?

"hello": match
"hello world": no match
"hello, world": no match
" hello": no match
"hello ": match


// Regex to match a string that does not contain any whitespaces nor the characters ,{}
// TODO c++17: Move to static constexpr std::string_view
string rse = "[^,\\{\\}\\s]*\\s*";

// Regex to match a path: a string that can't be empty and does not contain any space nor the characters *?<>|:",
// TODO c++17: Move to static constexpr std::string_view
string rp = "[^*?<>\\|:\";,\\s]+\\s*";

// Regex to match a path: a string that does not contain any space nor the characters *?<>|:",
// TODO c++17: Move to static constexpr std::string_view
string rpe = "[^*?<>\\|:\";,\\s]*\\s*";

// Regex to match a R1Tensor
// TODO c++17: Move to static constexpr std::string_view
string r1 = "\\s*\\{\\s*(" + rr + ",\\s*){2}" + rr + "\\s*\\}";

// Regex to match a R2SymTensor
// TODO c++17: Move to static constexpr std::string_view
string r2s = "\\s*\\{\\s*(" + rr + ",\\s*){5}" + rr + "\\s*\\}";

// Build master list of regexes
Expand Down Expand Up @@ -679,11 +697,11 @@ class rtTypes
{"real32_array3d", constructArrayRegex( rr, 3 )},
{"real64_array3d", constructArrayRegex( rr, 3 )},
{"real64_array4d", constructArrayRegex( rr, 4 )},
{"string", rs},
{"path", rs},
{"string", rse},
{"path", rpe},
{"string_array", constructArrayRegex( rs, 1 )},
{"path_array", constructArrayRegex( rs, 1 )},
{"mapPair", rs},
{"path_array", constructArrayRegex( rp, 1 )},
{"mapPair", rse},
{"geos_dataRepository_PlotLevel", ri}
};
};
Expand Down
39 changes: 23 additions & 16 deletions src/coreComponents/dataRepository/Wrapper.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -611,25 +611,32 @@ class Wrapper final : public WrapperBase
InputFlags const inputFlag = getInputFlag();
if( inputFlag >= InputFlags::OPTIONAL )
{
if( inputFlag == InputFlags::REQUIRED || !hasDefaultValue() )
try
{
m_successfulReadFromInput = xmlWrapper::readAttributeAsType( reference(),
getName(),
targetNode,
inputFlag == InputFlags::REQUIRED );
GEOS_THROW_IF( !m_successfulReadFromInput,
GEOS_FMT( "XML Node '{}' with name='{}' is missing required attribute '{}'."
"Available options are:\n{}\nFor more details, please refer to documentation at:\n"
"http://geosx-geosx.readthedocs-hosted.com/en/latest/docs/sphinx/userGuide/Index.html",
targetNode.path(), targetNode.attribute( "name" ).value(), getName(), dumpInputOptions( true ) ),
InputError );
if( inputFlag == InputFlags::REQUIRED || !hasDefaultValue() )
{
m_successfulReadFromInput = xmlWrapper::readAttributeAsType( reference(),
getName(),
targetNode,
inputFlag == InputFlags::REQUIRED );
GEOS_THROW_IF( !m_successfulReadFromInput,
GEOS_FMT( "XML Node '{}' with name='{}' is missing required attribute '{}'."
"Available options are:\n{}\nFor more details, please refer to documentation at:\n"
"http://geosx-geosx.readthedocs-hosted.com/en/latest/docs/sphinx/userGuide/Index.html",
targetNode.path(), targetNode.attribute( "name" ).value(), getName(), dumpInputOptions( true ) ),
InputError );
}
else
{
m_successfulReadFromInput = xmlWrapper::readAttributeAsType( reference(),
getName(),
targetNode,
getDefaultValueStruct() );
}
}
else
catch( std::exception const & ex )
{
m_successfulReadFromInput = xmlWrapper::readAttributeAsType( reference(),
getName(),
targetNode,
getDefaultValueStruct() );
processInputException( ex, targetNode );
}

return true;
Expand Down
12 changes: 12 additions & 0 deletions src/coreComponents/dataRepository/WrapperBase.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,18 @@ int WrapperBase::setTotalviewDisplay() const
}
#endif

void WrapperBase::processInputException( std::exception const & ex,
xmlWrapper::xmlNode const & targetNode ) const
{
xmlWrapper::xmlAttribute const & attribute = targetNode.attribute( getName().c_str() );
string const inputStr = string( attribute.value() );
string subExStr = string( "***** XML parsing error in " ) + targetNode.path() +
" (name=" + targetNode.attribute( "name" ).value() + ")/" + attribute.name() +
"\n***** Input value: '" + inputStr + "'\n";

throw InputError( subExStr + ex.what() );
}


}
} /* namespace geos */
Expand Down
7 changes: 7 additions & 0 deletions src/coreComponents/dataRepository/WrapperBase.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -612,6 +612,13 @@ class WrapperBase

/// @endcond

/**
* @brief Helper method to process an exception that has been thrown during xml parsing.
* @param ex The caught exception.
* @param targetNode The node from which this Group is interpreted.
*/
void processInputException( std::exception const & ex, xmlWrapper::xmlNode const & targetNode ) const;

protected:

/// Name of the object that is being wrapped
Expand Down
10 changes: 5 additions & 5 deletions src/coreComponents/schema/schema.xsd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,17 +85,17 @@
</xsd:simpleType>
<xsd:simpleType name="mapPair">
<xsd:restriction base="xsd:string">
<xsd:pattern value=".*[\[\]`$].*|[^,\{\}]*" />
<xsd:pattern value=".*[\[\]`$].*|[^,\{\}\s]*\s*" />
</xsd:restriction>
</xsd:simpleType>
<xsd:simpleType name="path">
<xsd:restriction base="xsd:string">
<xsd:pattern value=".*[\[\]`$].*|[^,\{\}]*" />
<xsd:pattern value=".*[\[\]`$].*|[^*?&lt;&gt;\|:&quot;;,\s]*\s*" />
</xsd:restriction>
</xsd:simpleType>
<xsd:simpleType name="path_array">
<xsd:restriction base="xsd:string">
<xsd:pattern value=".*[\[\]`$].*|\{\s*(([^,\{\}]*,\s*)*[^,\{\}]*)?\s*\}" />
<xsd:pattern value=".*[\[\]`$].*|\{\s*(([^*?&lt;&gt;\|:&quot;;,\s]+\s*,\s*)*[^*?&lt;&gt;\|:&quot;;,\s]+\s*)?\s*\}" />
</xsd:restriction>
</xsd:simpleType>
<xsd:simpleType name="real32">
Expand Down Expand Up @@ -145,12 +145,12 @@
</xsd:simpleType>
<xsd:simpleType name="string">
<xsd:restriction base="xsd:string">
<xsd:pattern value=".*[\[\]`$].*|[^,\{\}]*" />
<xsd:pattern value=".*[\[\]`$].*|[^,\{\}\s]*\s*" />
</xsd:restriction>
</xsd:simpleType>
<xsd:simpleType name="string_array">
<xsd:restriction base="xsd:string">
<xsd:pattern value=".*[\[\]`$].*|\{\s*(([^,\{\}]*,\s*)*[^,\{\}]*)?\s*\}" />
<xsd:pattern value=".*[\[\]`$].*|\{\s*(([^,\{\}\s]+\s*,\s*)*[^,\{\}\s]+\s*)?\s*\}" />
</xsd:restriction>
</xsd:simpleType>
<xsd:element name="Problem" type="ProblemType" />
Expand Down
10 changes: 5 additions & 5 deletions src/coreComponents/schema/schema.xsd.other
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,17 +85,17 @@
</xsd:simpleType>
<xsd:simpleType name="mapPair">
<xsd:restriction base="xsd:string">
<xsd:pattern value=".*[\[\]`$].*|[^,\{\}]*" />
<xsd:pattern value=".*[\[\]`$].*|[^,\{\}\s]*\s*" />
</xsd:restriction>
</xsd:simpleType>
<xsd:simpleType name="path">
<xsd:restriction base="xsd:string">
<xsd:pattern value=".*[\[\]`$].*|[^,\{\}]*" />
<xsd:pattern value=".*[\[\]`$].*|[^*?&lt;&gt;\|:&quot;;,\s]*\s*" />
</xsd:restriction>
</xsd:simpleType>
<xsd:simpleType name="path_array">
<xsd:restriction base="xsd:string">
<xsd:pattern value=".*[\[\]`$].*|\{\s*(([^,\{\}]*,\s*)*[^,\{\}]*)?\s*\}" />
<xsd:pattern value=".*[\[\]`$].*|\{\s*(([^*?&lt;&gt;\|:&quot;;,\s]+\s*,\s*)*[^*?&lt;&gt;\|:&quot;;,\s]+\s*)?\s*\}" />
</xsd:restriction>
</xsd:simpleType>
<xsd:simpleType name="real32">
Expand Down Expand Up @@ -145,12 +145,12 @@
</xsd:simpleType>
<xsd:simpleType name="string">
<xsd:restriction base="xsd:string">
<xsd:pattern value=".*[\[\]`$].*|[^,\{\}]*" />
<xsd:pattern value=".*[\[\]`$].*|[^,\{\}\s]*\s*" />
</xsd:restriction>
</xsd:simpleType>
<xsd:simpleType name="string_array">
<xsd:restriction base="xsd:string">
<xsd:pattern value=".*[\[\]`$].*|\{\s*(([^,\{\}]*,\s*)*[^,\{\}]*)?\s*\}" />
<xsd:pattern value=".*[\[\]`$].*|\{\s*(([^,\{\}\s]+\s*,\s*)*[^,\{\}\s]+\s*)?\s*\}" />
</xsd:restriction>
</xsd:simpleType>
<xsd:element name="Problem" type="ProblemType" />
Expand Down