App: Improve Metadata error handling

Report XML parse exception details.

src/App/Metadata.cpp

@@ -24,6 +24,7 @@
 
 #ifndef _PreComp_
 # include <memory>
+# include <sstream>
 #endif
 
 #include "Metadata.h"
@@ -55,6 +56,34 @@ using namespace App;
 namespace fs = boost::filesystem;
 XERCES_CPP_NAMESPACE_USE
 
+namespace MetadataInternal {
+    class XMLErrorHandler : public HandlerBase {
+        void warning(const SAXParseException& toCatch)
+        {
+            // Don't deal with warnings at all
+        }
+
+        void error(const SAXParseException& toCatch)
+        {
+            std::stringstream message;
+            message << "Error at file \"" << StrX(toCatch.getSystemId())
+                << "\", line " << toCatch.getLineNumber()
+                << ", column " << toCatch.getColumnNumber()
+                << "\n   Message: " << StrX(toCatch.getMessage()) << std::endl;
+            throw Base::XMLBaseException(message.str());
+        }
+
+        void fatalError(const SAXParseException& toCatch)
+        {
+            std::stringstream message;
+            message << "Fatal error at file \"" << StrX(toCatch.getSystemId())
+                << "\", line " << toCatch.getLineNumber()
+                << ", column " << toCatch.getColumnNumber()
+                << "\n   Message: " << StrX(toCatch.getMessage()) << std::endl;
+            throw Base::XMLBaseException(message.str());
+        }
+    };
+}
 
 Metadata::Metadata(const fs::path& metadataFile)
 {
@@ -65,7 +94,7 @@ Metadata::Metadata(const fs::path& metadataFile)
     _parser->setValidationScheme(XercesDOMParser::Val_Never);
     _parser->setDoNamespaces(true);
 
-    auto errHandler = std::make_unique<HandlerBase>();
+    auto errHandler = std::make_unique<MetadataInternal::XMLErrorHandler>();
     _parser->setErrorHandler(errHandler.get());
 
     _parser->parse(metadataFile.string().c_str());

src/App/MetadataPyImp.cpp

@@ -29,6 +29,7 @@
 #include "MetadataPy.cpp"
 
 using namespace Base;
+XERCES_CPP_NAMESPACE_USE
 
 // Returns a string which represents the object e.g. when printed in Python
 std::string MetadataPy::representation(void) const
@@ -70,6 +71,24 @@ int MetadataPy::PyInit(PyObject* args, PyObject* /*kwd*/)
             setTwinPointer(md);
             return 0;
         }
+        catch (const Base::XMLBaseException& e) {
+            PyErr_SetString(Base::BaseExceptionFreeCADError, e.what());
+            return -1;
+        }
+        catch (const XMLException& toCatch) {
+            char* message = XMLString::transcode(toCatch.getMessage());
+            std::string what = message;
+            XMLString::release(&message);
+            PyErr_SetString(Base::BaseExceptionFreeCADError, what.c_str());
+            return -1;
+        }
+        catch (const DOMException& toCatch) {
+            char* message = XMLString::transcode(toCatch.getMessage());
+            std::string what = message;
+            XMLString::release(&message);
+            PyErr_SetString(Base::BaseExceptionFreeCADError, what.c_str());
+            return -1;
+        }
         catch (...) {
             PyErr_SetString(Base::BaseExceptionFreeCADError, "Failed to create Metadata object");
             return -1;

src/Mod/Test/CMakeLists.txt

@@ -18,6 +18,9 @@ SET(Test_SRCS
 
 SET(TestData_SRCS
     TestData/basic_metadata.xml
+    TestData/bad_root_node.xml
+    TestData/bad_xml.xml
+    TestData/bad_version.xml
 )
 
 SOURCE_GROUP("" FILES ${Test_SRCS} ${TestData_SRCS})

src/Mod/Test/Metadata.py

@@ -31,6 +31,40 @@ class TestMetadata(unittest.TestCase):
     def setUp(self):
         self.test_dir = os.path.join(FreeCAD.getHomePath(), "Mod", "Test", "TestData")
 
+    def test_xml_constructor(self):
+        try:
+            filename = os.path.join(self.test_dir, "basic_metadata.xml")
+            md = FreeCAD.Metadata(filename)
+        except Exception:
+            self.fail("Metadata construction from XML file failed")
+
+        try:
+            filename = os.path.join(self.test_dir, "bad_root_node.xml")
+            md = FreeCAD.Metadata(filename)
+        except Exception as e:
+            FreeCAD.Console.PrintMessage(str(e) + "\n")
+            pass
+        else:
+            self.fail("Metadata construction from XML file with bad root node did not raise an exception")
+
+        try:
+            filename = os.path.join(self.test_dir, "bad_xml.xml")
+            md = FreeCAD.Metadata(filename)
+        except Exception as e:
+            FreeCAD.Console.PrintMessage(str(e) + "\n")
+            pass
+        else:
+            self.fail("Metadata construction from invalid XML file did not raise an exception")
+
+        try:
+            filename = os.path.join(self.test_dir, "bad_version.xml")
+            md = FreeCAD.Metadata(filename)
+        except Exception as e:
+            FreeCAD.Console.PrintMessage(str(e) + "\n")
+            pass
+        else:
+            self.fail("Metadata construction from XML file with bad version did not raise an exception")
+
     def test_toplevel_tags(self):
         filename = os.path.join(self.test_dir, "basic_metadata.xml")
         md = FreeCAD.Metadata(filename)

src/Mod/Test/TestData/bad_root_node.xml

@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="utf-8" standalone="no" ?>
+<notpackage format="1" xmlns="https://wiki.freecad.org/Package_Metadata">
+  <name>Bad root node</name>
+  <description>The root node is not called "package".</description>
+  <version>1.0.0</version>
+</notpackage>

src/Mod/Test/TestData/bad_version.xml

@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="utf-8" standalone="no" ?>
+<package format="2" xmlns="https://wiki.freecad.org/Package_Metadata">
+	<name>Bad format</name>
+	<description>There is no such thing as format version 2 (yet).</description>
+	<version>1.0.0</version>
+</package>

src/Mod/Test/TestData/bad_xml.xml

@@ -0,0 +1,5 @@
+<?xml version="1.0" encoding="utf-8" standalone="no" ?>
+<package format="1" xmlns="https://wiki.freecad.org/Package_Metadata">
+  <name>Bad XML</notname>
+  <description>The name tag is not closed properly.</description>
+</package>