diff --git a/include/sdf/Error.hh b/include/sdf/Error.hh index 0aace5ef7..9664cc850 100644 --- a/include/sdf/Error.hh +++ b/include/sdf/Error.hh @@ -183,6 +183,9 @@ namespace sdf /// \brief The joint axis mimic does not refer to a valid joint in the /// current scope. JOINT_AXIS_MIMIC_INVALID, + + /// \brief Error at the XML level. + XML_ERROR, }; class SDFORMAT_VISIBLE Error diff --git a/src/Converter.cc b/src/Converter.cc index 8947d3333..41d550df1 100644 --- a/src/Converter.cc +++ b/src/Converter.cc @@ -264,7 +264,7 @@ void Converter::ConvertImpl(tinyxml2::XMLElement *_elem, } else if (name == "copy") { - Move(_elem, childElem, true); + Move(_elem, childElem, true, _errors); } else if (name == "map") { @@ -272,7 +272,7 @@ void Converter::ConvertImpl(tinyxml2::XMLElement *_elem, } else if (name == "move") { - Move(_elem, childElem, false); + Move(_elem, childElem, false, _errors); } else if (name == "add") { @@ -923,7 +923,8 @@ void Converter::Map(tinyxml2::XMLElement *_elem, ///////////////////////////////////////////////// void Converter::Move(tinyxml2::XMLElement *_elem, tinyxml2::XMLElement *_moveElem, - const bool _copy) + const bool _copy, + sdf::Errors &_errors) { SDF_ASSERT(_elem != NULL, "SDF element is NULL"); SDF_ASSERT(_moveElem != NULL, "Move element is NULL"); @@ -1024,7 +1025,8 @@ void Converter::Move(tinyxml2::XMLElement *_elem, if (toElemStr && !toAttrStr) { - tinyxml2::XMLNode *cloned = DeepClone(moveFrom->GetDocument(), moveFrom); + tinyxml2::XMLNode *cloned = DeepClone(_errors, moveFrom->GetDocument(), + moveFrom); tinyxml2::XMLElement *moveTo = static_cast(cloned); moveTo->SetValue(toName); diff --git a/src/Converter.hh b/src/Converter.hh index e992b124c..cfba5413f 100644 --- a/src/Converter.hh +++ b/src/Converter.hh @@ -108,9 +108,11 @@ namespace sdf /// \param[in] _moveElem A 'convert' element that describes the move /// operation. /// \param[in] _copy True to copy the element + /// \param[out] _errors Vector of errors. private: static void Move(tinyxml2::XMLElement *_elem, tinyxml2::XMLElement *_moveElem, - const bool _copy); + const bool _copy, + sdf::Errors &_errors); /// \brief Add an element or attribute to an element. /// \param[in] _elem The element to receive the value. diff --git a/src/Converter_TEST.cc b/src/Converter_TEST.cc index 443774017..d56cba30a 100644 --- a/src/Converter_TEST.cc +++ b/src/Converter_TEST.cc @@ -3370,7 +3370,8 @@ TEST(Converter, World_17_to_18) ASSERT_TRUE(errors.empty()); // Compare converted xml with expected - std::string convertedXmlStr = ElementToString(xmlDoc.RootElement()); + std::string convertedXmlStr = ElementToString(errors, xmlDoc.RootElement()); + ASSERT_TRUE(errors.empty()); ASSERT_FALSE(convertedXmlStr.empty()); std::string expectedXmlStr = R"( @@ -3393,7 +3394,9 @@ TEST(Converter, World_17_to_18) tinyxml2::XMLDocument expectedXmlDoc; expectedXmlDoc.Parse(expectedXmlStr.c_str()); - EXPECT_EQ(ElementToString(expectedXmlDoc.RootElement()), convertedXmlStr); + EXPECT_EQ(ElementToString(errors, expectedXmlDoc.RootElement()), + convertedXmlStr); + ASSERT_TRUE(errors.empty()); // Check some basic elements tinyxml2::XMLElement *convertedElem = xmlDoc.FirstChildElement(); @@ -3509,7 +3512,8 @@ TEST(Converter, World_17_to_18) ASSERT_TRUE(errors.empty()); // Compare converted xml with expected - convertedXmlStr = ElementToString(xmlDoc.RootElement()); + convertedXmlStr = ElementToString(errors, xmlDoc.RootElement()); + ASSERT_TRUE(errors.empty()); ASSERT_FALSE(convertedXmlStr.empty()); expectedXmlStr = R"( @@ -3577,7 +3581,9 @@ TEST(Converter, World_17_to_18) expectedXmlDoc.Clear(); expectedXmlDoc.Parse(expectedXmlStr.c_str()); - EXPECT_EQ(ElementToString(expectedXmlDoc.RootElement()), convertedXmlStr); + EXPECT_EQ(ElementToString(errors, expectedXmlDoc.RootElement()), + convertedXmlStr); + ASSERT_TRUE(errors.empty()); // ------- Another flattened world in 1.7 format @@ -3616,7 +3622,8 @@ TEST(Converter, World_17_to_18) EXPECT_TRUE(buffer.str().empty()) << buffer.str(); // Compare converted xml with expected - convertedXmlStr = ElementToString(xmlDoc.RootElement()); + convertedXmlStr = ElementToString(errors, xmlDoc.RootElement()); + ASSERT_TRUE(errors.empty()); ASSERT_FALSE(convertedXmlStr.empty()); expectedXmlStr = R"( @@ -3655,7 +3662,9 @@ TEST(Converter, World_17_to_18) expectedXmlDoc.Clear(); expectedXmlDoc.Parse(expectedXmlStr.c_str()); - EXPECT_EQ(ElementToString(expectedXmlDoc.RootElement()), convertedXmlStr); + EXPECT_EQ(ElementToString(errors, expectedXmlDoc.RootElement()), + convertedXmlStr); + ASSERT_TRUE(errors.empty()); // Check some basic elements convertedElem = xmlDoc.FirstChildElement(); diff --git a/src/ParamPassing.cc b/src/ParamPassing.cc index 1b80d09cf..5bf0c97a9 100644 --- a/src/ParamPassing.cc +++ b/src/ParamPassing.cc @@ -52,7 +52,7 @@ void updateParams(const ParserConfig &_config, _errors.push_back({ErrorCode::ATTRIBUTE_MISSING, "Element identifier requires an element_id attribute, but the " "element_id is not set. Skipping element alteration:\n" - + ElementToString(childElemXml) + + ElementToString(_errors, childElemXml) }); continue; } @@ -67,7 +67,7 @@ void updateParams(const ParserConfig &_config, _errors.push_back({ErrorCode::ATTRIBUTE_INVALID, "Missing name after double colons in element identifier. " "Skipping element alteration:\n" - + ElementToString(childElemXml) + + ElementToString(_errors, childElemXml) }); continue; } @@ -83,7 +83,7 @@ void updateParams(const ParserConfig &_config, { _errors.push_back({ErrorCode::ATTRIBUTE_INVALID, "Action [" + actionStr + "] is not a valid action. Skipping " - "element alteration:\n" + ElementToString(childElemXml) + "element alteration:\n" + ElementToString(_errors, childElemXml) }); continue; } @@ -102,7 +102,7 @@ void updateParams(const ParserConfig &_config, _errors.push_back({ErrorCode::ATTRIBUTE_MISSING, "Element to be added is missing a 'name' attribute. " "Skipping element addition:\n" - + ElementToString(childElemXml) + + ElementToString(_errors, childElemXml) }); continue; } @@ -112,7 +112,7 @@ void updateParams(const ParserConfig &_config, { _errors.push_back({ErrorCode::ATTRIBUTE_INVALID, "The 'name' attribute can not be empty. Skipping element addition:\n" - + ElementToString(childElemXml) + + ElementToString(_errors, childElemXml) }); continue; } @@ -129,7 +129,7 @@ void updateParams(const ParserConfig &_config, + " element_id='" + childElemXml->Attribute("element_id") + "'> because element already exists in included model. " + "Skipping element addition:\n" - + ElementToString(childElemXml) + + ElementToString(_errors, childElemXml) }); continue; } @@ -157,7 +157,8 @@ void updateParams(const ParserConfig &_config, _errors.push_back({ErrorCode::ELEMENT_MISSING, "Could not find element <" + std::string(childElemXml->Name()) + " element_id='" + childElemXml->Attribute("element_id") + "'>. " + - "Skipping element modification:\n" + ElementToString(childElemXml) + "Skipping element modification:\n" + + ElementToString(_errors, childElemXml) }); continue; } @@ -194,7 +195,7 @@ void updateParams(const ParserConfig &_config, { _errors.push_back({ErrorCode::ELEMENT_INVALID, "Unable to convert XML to SDF. Skipping element replacement:\n" - + ElementToString(childElemXml) + + ElementToString(_errors, childElemXml) }); continue; } @@ -355,7 +356,7 @@ ElementPtr initElementDescription(const tinyxml2::XMLElement *_xml, _errors.push_back({ErrorCode::ELEMENT_INVALID, "Element [" + std::string(_xml->Name()) + "] is not a defined " "SDF element. Skipping element alteration\n: " - + ElementToString(_xml) + + ElementToString(_errors, _xml) }); return nullptr; } @@ -384,7 +385,7 @@ void handleIndividualChildActions(const ParserConfig &_config, "Missing an action attribute. Skipping child element modification " "with parent <" + std::string(_childrenXml->Name()) + " element_id='" + std::string(_childrenXml->Attribute("element_id")) + "'>:\n" - + ElementToString(xmlChild) + + ElementToString(_errors, xmlChild) }); continue; } @@ -397,7 +398,7 @@ void handleIndividualChildActions(const ParserConfig &_config, "child element modification with parent <" + std::string(_childrenXml->Name()) + " element_id='" + std::string(_childrenXml->Attribute("element_id")) + "'>:\n" - + ElementToString(xmlChild) + + ElementToString(_errors, xmlChild) }); continue; } @@ -411,7 +412,7 @@ void handleIndividualChildActions(const ParserConfig &_config, "Could not find element. Skipping child element removal " "with parent <" + std::string(_childrenXml->Name()) + " element_id='" + std::string(_childrenXml->Attribute("element_id")) + "'>:\n" - + ElementToString(xmlChild) + + ElementToString(_errors, xmlChild) }); } else @@ -430,7 +431,7 @@ void handleIndividualChildActions(const ParserConfig &_config, "Could not find element. Skipping child element modification " "with parent <" + std::string(_childrenXml->Name()) + " element_id='" + std::string(_childrenXml->Attribute("element_id")) + "'>:\n" - + ElementToString(xmlChild) + + ElementToString(_errors, xmlChild) }); } else @@ -455,7 +456,7 @@ void handleIndividualChildActions(const ParserConfig &_config, "child element modification with parent <" + std::string(_childrenXml->Name()) + " element_id='" + std::string(_childrenXml->Attribute("element_id")) + "'>:\n" - + ElementToString(xmlChild) + + ElementToString(_errors, xmlChild) }); continue; } @@ -468,7 +469,7 @@ void handleIndividualChildActions(const ParserConfig &_config, "Unable to convert XML to SDF. Skipping child element alteration " "with parent <" + std::string(_childrenXml->Name()) + " element_id='" + std::string(_childrenXml->Attribute("element_id")) + "'>:\n" - + ElementToString(xmlChild) + + ElementToString(_errors, xmlChild) }); continue; } @@ -486,7 +487,7 @@ void handleIndividualChildActions(const ParserConfig &_config, "Could not find element. Skipping child element replacement " "with parent <" + std::string(_childrenXml->Name()) + " element_id='" + std::string(_childrenXml->Attribute("element_id")) + "'>:\n" - + ElementToString(xmlChild) + + ElementToString(_errors, xmlChild) }); continue; } @@ -498,7 +499,7 @@ void handleIndividualChildActions(const ParserConfig &_config, "Replacement element is missing a 'name' attribute. " "Skipping element replacement <" + std::string(_childrenXml->Name()) + " element_id='" + std::string(_childrenXml->Attribute("element_id")) - + "'>:\n" + ElementToString(xmlChild) + + "'>:\n" + ElementToString(_errors, xmlChild) }); continue; } @@ -525,7 +526,7 @@ void add(const ParserConfig &_config, const std::string &_source, { _errors.push_back({ErrorCode::ELEMENT_INVALID, "Unable to convert XML to SDF. Skipping element addition:\n" - + ElementToString(_childXml) + + ElementToString(_errors, _childXml) }); } } @@ -557,7 +558,8 @@ void modifyAttributes(tinyxml2::XMLElement *_xml, { _errors.push_back({ErrorCode::ATTRIBUTE_INVALID, "Attribute [" + attrName + "] is invalid. " - "Skipping attribute modification in:\n" + ElementToString(_xml) + "Skipping attribute modification in:\n" + + ElementToString(_errors, _xml) }); continue; } @@ -582,7 +584,7 @@ void modifyChildren(tinyxml2::XMLElement *_xml, { _errors.push_back({ErrorCode::ELEMENT_MISSING, "Could not find element [" + elemName + "]. " - "Skipping modification for:\n" + ElementToString(_xml) + "Skipping modification for:\n" + ElementToString(_errors, _xml) }); continue; } @@ -599,7 +601,7 @@ void modifyChildren(tinyxml2::XMLElement *_xml, _errors.push_back({ErrorCode::ELEMENT_INVALID, "Value [" + std::string(xmlChild->GetText()) + "] for element [" + elemName + "] is invalid. Skipping modification for:\n" - + ElementToString(_xml) + + ElementToString(_errors, _xml) }); continue; } @@ -620,9 +622,9 @@ void modifyChildren(tinyxml2::XMLElement *_xml, // sdf has child elements but no children were specified in xml std::stringstream ss; ss << "No modifications for element " - << ElementToString(xmlChild) + << ElementToString(_errors, xmlChild) << " provided, skipping modification for:\n" - << ElementToString(_xml); + << ElementToString(_errors, _xml); Error err(ErrorCode::WARNING, ss.str()); enforceConfigurablePolicyCondition( _config.WarningsPolicy(), err, _errors); @@ -650,7 +652,7 @@ void modify(tinyxml2::XMLElement *_xml, const sdf::ParserConfig &_config, _errors.push_back({ErrorCode::ELEMENT_INVALID, "Value [" + std::string(_xml->GetText()) + "] for element [" + std::string(_xml->Name()) + "] is invalid. Skipping modification for:\n" - + ElementToString(_xml) + + ElementToString(_errors, _xml) }); } } @@ -688,7 +690,7 @@ void remove(const tinyxml2::XMLElement *_xml, const sdf::ParserConfig &_config, + std::string(xmlParent->Name()) + " element_id='" + std::string(xmlParent->Attribute("element_id")) + "'> with parent <" + std::string(_xml->Name()) + ">:\n" - + ElementToString(xmlChild) + + ElementToString(_errors, xmlChild) }); continue; } diff --git a/src/XmlUtils.cc b/src/XmlUtils.cc index 4dbceab71..5d3709707 100644 --- a/src/XmlUtils.cc +++ b/src/XmlUtils.cc @@ -15,7 +15,9 @@ * */ #include +#include +#include "Utils.hh" #include "XmlUtils.hh" #include "sdf/Console.hh" @@ -27,27 +29,41 @@ inline namespace SDF_VERSION_NAMESPACE { ///////////////////////////////////////////////// tinyxml2::XMLNode *DeepClone(tinyxml2::XMLDocument *_doc, const tinyxml2::XMLNode *_src) +{ + sdf::Errors errors; + tinyxml2::XMLNode *result = DeepClone(errors, _doc, _src); + sdf::throwOrPrintErrors(errors); + return result; +} + +///////////////////////////////////////////////// +tinyxml2::XMLNode *DeepClone(sdf::Errors &_errors, + tinyxml2::XMLDocument *_doc, + const tinyxml2::XMLNode *_src) { if (_src == nullptr) { - sdferr << "Pointer to XML node _src is NULL\n"; + _errors.push_back({sdf::ErrorCode::XML_ERROR, + "Pointer to XML node _src is NULL"}); return nullptr; } tinyxml2::XMLNode *copy = _src->ShallowClone(_doc); if (copy == nullptr) { - sdferr << "Failed to clone node " << _src->Value() << "\n"; + _errors.push_back({sdf::ErrorCode::XML_ERROR, + "Failed to clone node " + std::string(_src->Value())}); return nullptr; } for (const tinyxml2::XMLNode *node = _src->FirstChild(); node != nullptr; node = node->NextSibling()) { - auto *childCopy = DeepClone(_doc, node); + auto *childCopy = DeepClone(_errors, _doc, node); if (childCopy == nullptr) { - sdferr << "Failed to clone child " << node->Value() << "\n"; + _errors.push_back({sdf::ErrorCode::XML_ERROR, + "Failed to clone child " + std::string(node->Value())}); return nullptr; } copy->InsertEndChild(childCopy); @@ -57,11 +73,13 @@ tinyxml2::XMLNode *DeepClone(tinyxml2::XMLDocument *_doc, } ///////////////////////////////////////////////// -std::string ElementToString(const tinyxml2::XMLElement *_elem) +std::string ElementToString(sdf::Errors &_errors, + const tinyxml2::XMLElement *_elem) { if (_elem == nullptr) { - sdferr << "Pointer to XML Element _elem is nullptr\n"; + _errors.push_back({sdf::ErrorCode::XML_ERROR, + "Pointer to XML Element _elem is nullptr"}); return ""; } diff --git a/src/XmlUtils.hh b/src/XmlUtils.hh index 7ad0c30d4..522458aad 100644 --- a/src/XmlUtils.hh +++ b/src/XmlUtils.hh @@ -41,10 +41,26 @@ namespace sdf tinyxml2::XMLNode *DeepClone(tinyxml2::XMLDocument *_doc, const tinyxml2::XMLNode *_src); + /// \brief Perform a deep copy of an XML Node + /// + /// This copies an XMLNode _src and all of its decendants + /// into a specified XMLDocument. + /// + /// \param[out] _errors Vector of errors + /// \param[in] _doc Document in which to place the copied node + /// \param[in] _src The node to deep copy + /// \returns The newly copied node upon success OR + /// nullptr if an error occurs. + tinyxml2::XMLNode *DeepClone(sdf::Errors &_errors, + tinyxml2::XMLDocument *_doc, + const tinyxml2::XMLNode *_src); + /// \brief Converts the XML Element to a string + /// \param[out] _errors Vector of errors /// \param[in] _elem Element to be converted /// \return The string representation - std::string ElementToString(const tinyxml2::XMLElement *_elem); + std::string ElementToString(sdf::Errors &_errors, + const tinyxml2::XMLElement *_elem); } } #endif diff --git a/src/XmlUtils_TEST.cc b/src/XmlUtils_TEST.cc index aef79262d..4190b058b 100644 --- a/src/XmlUtils_TEST.cc +++ b/src/XmlUtils_TEST.cc @@ -37,7 +37,9 @@ TEST(XMLUtils, DeepClone) ASSERT_EQ(tinyxml2::XML_SUCCESS, ret); auto root = oldDoc.FirstChild(); - auto newRoot = sdf::DeepClone(&newDoc, root); + sdf::Errors errors; + auto newRoot = sdf::DeepClone(errors, &newDoc, root); + EXPECT_TRUE(errors.empty()) << errors; EXPECT_STREQ("document", newRoot->ToElement()->Name()); @@ -53,3 +55,48 @@ TEST(XMLUtils, DeepClone) auto childB_text = newChildB->ToElement()->GetText(); EXPECT_STREQ("Hello World", childB_text); } + +///////////////////////////////////////////////// +TEST(XMLUtils, InvalidDeepClone) +{ + sdf::Errors errors; + auto newRoot = sdf::DeepClone(errors, nullptr, nullptr); + EXPECT_EQ(nullptr, newRoot); + EXPECT_EQ(1u, errors.size()) << errors; + ASSERT_FALSE(errors.empty()); + EXPECT_EQ(errors[0].Code(), sdf::ErrorCode::XML_ERROR); +} + +///////////////////////////////////////////////// +TEST(XMLUtils, ElementToString) +{ + tinyxml2::XMLDocument doc; + + std::string docXml = +R"( + + Hello World + + +)"; + + auto ret = doc.Parse(docXml.c_str()); + ASSERT_EQ(tinyxml2::XML_SUCCESS, ret); + + auto root = doc.FirstChild(); + sdf::Errors errors; + std::string docString = sdf::ElementToString(errors, root->ToElement()); + EXPECT_TRUE(errors.empty()) << errors; + EXPECT_EQ(docXml, docString); +} + +///////////////////////////////////////////////// +TEST(XMLUtils, InvalidElementToString) +{ + sdf::Errors errors; + std::string docString = sdf::ElementToString(errors, nullptr); + EXPECT_TRUE(docString.empty()); + EXPECT_EQ(1u, errors.size()) << errors; + ASSERT_FALSE(errors.empty()); + EXPECT_EQ(errors[0].Code(), sdf::ErrorCode::XML_ERROR); +}