diff --git a/src/ParamPassing.cc b/src/ParamPassing.cc index cea6e8b77..1b80d09cf 100644 --- a/src/ParamPassing.cc +++ b/src/ParamPassing.cc @@ -24,6 +24,7 @@ #include "ParamPassing.hh" #include "parser_private.hh" +#include "Utils.hh" #include "XmlUtils.hh" namespace sdf @@ -176,11 +177,11 @@ void updateParams(const ParserConfig &_config, } else if (actionStr == "modify") { - modify(childElemXml, elem, _errors); + modify(childElemXml, _config, elem, _errors); } else if (actionStr == "remove") { - remove(childElemXml, elem, _errors); + remove(childElemXml, _config, elem, _errors); } else if (actionStr == "replace") { @@ -292,6 +293,8 @@ bool isValidAction(const std::string &_action) ////////////////////////////////////////////////// ElementPtr getElementByName(const ElementPtr _elem, const tinyxml2::XMLElement *_xml, + const sdf::ParserConfig _config, + sdf::Errors &_errors, const bool _isModifyAction) { std::string elemName = _xml->Name(); @@ -324,10 +327,14 @@ ElementPtr getElementByName(const ElementPtr _elem, else if (elem->HasAttribute("name") && elem->GetAttribute("name")->GetRequired()) { - sdfwarn << "The original element [" << elemName << "] contains the " - << "attribute 'name' but none was provided in the element modifier." - << " The assumed element to be modified is: <" << elemName - << " name='" << elem->Get("name") << "'>\n"; + std::stringstream ss; + ss << "The original element [" << elemName << "] contains the " + << "attribute 'name' but none was provided in the element modifier." + << " The assumed element to be modified is: <" << elemName + << " name='" << elem->Get("name") << "'>"; + Error err(ErrorCode::WARNING, ss.str()); + enforceConfigurablePolicyCondition( + _config.WarningsPolicy(), err, _errors); } return elem; @@ -397,7 +404,7 @@ void handleIndividualChildActions(const ParserConfig &_config, if (actionStr == "remove") { - ElementPtr e = getElementByName(_elem, xmlChild); + ElementPtr e = getElementByName(_elem, xmlChild, _config, _errors); if (e == nullptr) { _errors.push_back({ErrorCode::ELEMENT_MISSING, @@ -409,14 +416,14 @@ void handleIndividualChildActions(const ParserConfig &_config, } else { - remove(xmlChild, e, _errors); + remove(xmlChild, _config, e, _errors); } continue; } else if (actionStr == "modify") { - ElementPtr e = getElementByName(_elem, xmlChild, true); + ElementPtr e = getElementByName(_elem, xmlChild, _config, _errors, true); if (e == nullptr) { _errors.push_back({ErrorCode::ELEMENT_MISSING, @@ -428,7 +435,7 @@ void handleIndividualChildActions(const ParserConfig &_config, } else { - modify(xmlChild, e, _errors); + modify(xmlChild, _config, e, _errors); } continue; @@ -472,7 +479,7 @@ void handleIndividualChildActions(const ParserConfig &_config, } else if (actionStr == "replace") { - ElementPtr e = getElementByName(_elem, xmlChild); + ElementPtr e = getElementByName(_elem, xmlChild, _config, _errors); if (e == nullptr) { _errors.push_back({ErrorCode::ELEMENT_MISSING, @@ -562,7 +569,8 @@ void modifyAttributes(tinyxml2::XMLElement *_xml, ////////////////////////////////////////////////// void modifyChildren(tinyxml2::XMLElement *_xml, - ElementPtr _elem, Errors &_errors) + const sdf::ParserConfig &_config, ElementPtr _elem, + Errors &_errors) { for (tinyxml2::XMLElement *xmlChild = _xml->FirstChildElement(); xmlChild; @@ -579,7 +587,8 @@ void modifyChildren(tinyxml2::XMLElement *_xml, continue; } - ElementPtr elemChild = getElementByName(_elem, xmlChild, true); + ElementPtr elemChild = getElementByName(_elem, xmlChild, + _config, _errors, true); ParamPtr paramChild = elemChild->GetValue(); if (xmlChild->GetText()) @@ -609,21 +618,26 @@ void modifyChildren(tinyxml2::XMLElement *_xml, else { // sdf has child elements but no children were specified in xml - sdfwarn << "No modifications for element " - << ElementToString(xmlChild) - << " provided, skipping modification for:\n" - << ElementToString(_xml) << "\n"; + std::stringstream ss; + ss << "No modifications for element " + << ElementToString(xmlChild) + << " provided, skipping modification for:\n" + << ElementToString(_xml); + Error err(ErrorCode::WARNING, ss.str()); + enforceConfigurablePolicyCondition( + _config.WarningsPolicy(), err, _errors); } } else { - modify(xmlChild, elemChild, _errors); + modify(xmlChild, _config, elemChild, _errors); } } } ////////////////////////////////////////////////// -void modify(tinyxml2::XMLElement *_xml, ElementPtr _elem, Errors &_errors) +void modify(tinyxml2::XMLElement *_xml, const sdf::ParserConfig &_config, + ElementPtr _elem, Errors &_errors) { modifyAttributes(_xml, _elem, _errors); @@ -643,12 +657,13 @@ void modify(tinyxml2::XMLElement *_xml, ElementPtr _elem, Errors &_errors) else { // modify children elements - modifyChildren(_xml, _elem, _errors); + modifyChildren(_xml, _config, _elem, _errors); } } ////////////////////////////////////////////////// -void remove(const tinyxml2::XMLElement *_xml, ElementPtr _elem, Errors &_errors) +void remove(const tinyxml2::XMLElement *_xml, const sdf::ParserConfig &_config, + ElementPtr _elem, Errors &_errors) { if (_xml->NoChildren()) { @@ -663,7 +678,7 @@ void remove(const tinyxml2::XMLElement *_xml, ElementPtr _elem, Errors &_errors) xmlChild; xmlChild = xmlChild->NextSiblingElement()) { - elemChild = getElementByName(_elem, xmlChild); + elemChild = getElementByName(_elem, xmlChild, _config, _errors); if (elemChild == nullptr) { const tinyxml2::XMLElement *xmlParent = _xml->Parent()->ToElement(); diff --git a/src/ParamPassing.hh b/src/ParamPassing.hh index 1572f6dda..ac077debb 100644 --- a/src/ParamPassing.hh +++ b/src/ParamPassing.hh @@ -85,6 +85,8 @@ namespace sdf /// \param[in] _elem The sdf (parent) element used to find the matching /// child element described in xml /// \param[in] _xml The xml element to find + /// \param[in] _config Custom parser configuration. + /// \param[out] _errors Vector of errros. /// \param[in] _isModifyAction Is true if the action is modify, the /// attribute 'name' may not be in the sdf element (i.e., may be a /// modified/added attribute such as //camera) @@ -92,6 +94,8 @@ namespace sdf /// element could not be found ElementPtr getElementByName(const ElementPtr _elem, const tinyxml2::XMLElement *_xml, + const sdf::ParserConfig _config, + sdf::Errors &_errors, const bool _isModifyAction = false); /// \brief Initialize an sdf element description from the xml element @@ -143,24 +147,31 @@ namespace sdf /// \brief Modifies the children elements of the included model /// \param[in] _xml Pointer to the xml element which contains the elements /// to be modified + /// \param[in] _config Custom parser configuration. /// \param[out] _elem The element from the included model to modify + /// \param[out] _errors Vector of errors. void modifyChildren(tinyxml2::XMLElement *_xml, - ElementPtr _elem, Errors &_errors); + const sdf::ParserConfig &_config, ElementPtr _elem, + Errors &_errors); /// \brief Modifies element values and/or attributes of an element from the /// included model /// \param[in] _xml Pointer to the xml element which contains the elements /// to be modified + /// \param[in] _config Custom parser configuration. /// \param[out] _elem The element from the included model to modify /// \param[out] _errors Captures errors found during parsing - void modify(tinyxml2::XMLElement *_xml, ElementPtr _elem, Errors &_errors); + void modify(tinyxml2::XMLElement *_xml, const sdf::ParserConfig &_config, + ElementPtr _elem, Errors &_errors); /// \brief Removes an element specified in xml /// \param[in] _xml Pointer to the xml element(s) to be removed from _elem + /// \param[in] _config Custom parser configuration. /// \param[out] _elem The element from the included model to remove /// elements from /// \param[out] _errors Captures errors found during parsing - void remove(const tinyxml2::XMLElement *_xml, ElementPtr _elem, + void remove(const tinyxml2::XMLElement *_xml, + const sdf::ParserConfig &_config, ElementPtr _elem, Errors &_errors); /// \brief Replace an element with another element diff --git a/src/ParamPassing_TEST.cc b/src/ParamPassing_TEST.cc index f9c9a5704..00f9ff48a 100644 --- a/src/ParamPassing_TEST.cc +++ b/src/ParamPassing_TEST.cc @@ -20,6 +20,7 @@ #include "ParamPassing.hh" #include "sdf/Element.hh" #include "sdf/parser.hh" +#include "test_utils.hh" ///////////////////////////////////////////////// TEST(ParamPassing, GetElement) @@ -91,3 +92,125 @@ TEST(ParamPassing, GetElement) "model::test_link::test_visual"); EXPECT_EQ(nullptr, paramPassElem); } + +//////////////////////////////////////// +// Test warnings outputs for GetElementByName +TEST(ParamPassing, GetElementByNameWarningOutput) +{ + std::stringstream buffer; + sdf::testing::RedirectConsoleStream redir( + sdf::Console::Instance()->GetMsgStream(), &buffer); + + std::ostringstream stream; + stream << "" + << "" + << " " + << " " + << " " + << ""; + + sdf::SDFPtr sdfParsed(new sdf::SDF()); + sdf::init(sdfParsed); + + sdf::Errors errors; + sdf::ParserConfig parserConfig; + parserConfig.SetWarningsPolicy(sdf::EnforcementPolicy::ERR); + sdf::readString(stream.str(), parserConfig, sdfParsed, errors); + EXPECT_TRUE(errors.empty()); + + std::ostringstream stream2; + stream2 << " " + << " "; + tinyxml2::XMLDocument doc; + doc.Parse(stream2.str().c_str()); + + sdf::ParamPassing::getElementByName(sdfParsed->Root(), + doc.FirstChildElement("model"), + parserConfig, + errors); + ASSERT_EQ(errors.size(), 1u); + EXPECT_EQ(errors[0].Code(), sdf::ErrorCode::WARNING); + EXPECT_NE(std::string::npos, errors[0].Message().find( + "The original element [model] contains the attribute 'name' but none was" + " provided in the element modifier. The assumed element to be modified " + "is: ")); + errors.clear(); + + // Check nothing has been printed + EXPECT_TRUE(buffer.str().empty()) << buffer.str(); + + parserConfig.SetWarningsPolicy(sdf::EnforcementPolicy::WARN); + sdf::ParamPassing::getElementByName(sdfParsed->Root(), + doc.FirstChildElement("model"), + parserConfig, + errors); + EXPECT_TRUE(errors.empty()); + // Check the warning has been printed + EXPECT_NE(std::string::npos, buffer.str().find( + "The original element [model] contains the attribute 'name' but none " + "was provided in the element modifier. The assumed element to be " + "modified is: ")); +} + +//////////////////////////////////////// +// Test warnings outputs for GetElementByName +TEST(ParamPassing, ModifyChildrenNameWarningOutput) +{ + std::stringstream buffer; + sdf::testing::RedirectConsoleStream redir( + sdf::Console::Instance()->GetMsgStream(), &buffer); + + std::ostringstream stream; + stream << "" + << "" + << " " + << " " + << " " + << ""; + + sdf::SDFPtr sdfParsed(new sdf::SDF()); + sdf::init(sdfParsed); + + sdf::Errors errors; + sdf::ParserConfig parserConfig; + parserConfig.SetWarningsPolicy(sdf::EnforcementPolicy::ERR); + sdf::readString(stream.str(), parserConfig, sdfParsed, errors); + EXPECT_TRUE(errors.empty()); + + std::ostringstream stream2; + stream2 << "" + << " " + << " " + << ""; + tinyxml2::XMLDocument doc; + doc.Parse(stream2.str().c_str()); + + sdf::ParamPassing::modifyChildren( + doc.FirstChildElement("sdf"), + parserConfig, + sdfParsed->Root(), + errors); + ASSERT_EQ(errors.size(), 1u); + EXPECT_EQ(errors[0].Code(), sdf::ErrorCode::WARNING); + EXPECT_NE(std::string::npos, errors[0].Message().find( + "No modifications for element \n provided, " + "skipping modification for:\n\n" + " \n")); + errors.clear(); + + // Check nothing has been printed + EXPECT_TRUE(buffer.str().empty()) << buffer.str(); + + parserConfig.SetWarningsPolicy(sdf::EnforcementPolicy::WARN); + sdf::ParamPassing::modifyChildren( + doc.FirstChildElement("sdf"), + parserConfig, + sdfParsed->Root(), + errors); + EXPECT_TRUE(errors.empty()); + // Check the warning has been printed + EXPECT_NE(std::string::npos, buffer.str().find( + "No modifications for element \n provided, " + "skipping modification for:\n\n" + " \n")); +}