From 2ac4e60ab5c23dcb2794a6a0088f87ace5b87ccb Mon Sep 17 00:00:00 2001
From: Chris Hennes <chennes@pioneerlibrarysystem.org>
Date: Sun, 13 Mar 2022 17:16:29 -0500
Subject: [PATCH] App: Fix Metadata Py constructors

Also adds unit tests for the corrected Copy and Default constructors.
---
 src/App/MetadataPyImp.cpp | 36 ++++++++++++++++--------------------
 src/Mod/Test/Metadata.py  | 14 ++++++++++++++
 2 files changed, 30 insertions(+), 20 deletions(-)

diff --git a/src/App/MetadataPyImp.cpp b/src/App/MetadataPyImp.cpp
index 6a8c3ea98d..68a9190a77 100644
--- a/src/App/MetadataPyImp.cpp
+++ b/src/App/MetadataPyImp.cpp
@@ -48,36 +48,32 @@ std::string MetadataPy::representation(void) const
     return str.str();
 }
 
-PyObject* MetadataPy::PyMake(struct _typeobject*, PyObject* args, PyObject*)  // Python wrapper
+PyObject* MetadataPy::PyMake(struct _typeobject*, PyObject*, PyObject*)  // Python wrapper
 {
-    // create a new instance of MetadataPy and the Twin object
-    const char* filename;
-    if (!PyArg_ParseTuple(args, "s", &filename))
-        return nullptr;
-    try {
-        auto md = new Metadata(filename);
-        return new MetadataPy(md);
-    }
-    catch (...) {
-        PyErr_SetString(Base::BaseExceptionFreeCADError, "Failed to create Metadata object");
-        return nullptr;
-    }
+    return new MetadataPy(nullptr);
 }
 
 // constructor method
 int MetadataPy::PyInit(PyObject* args, PyObject* /*kwd*/)
 {
     if (PyArg_ParseTuple(args, "")) {
+        setTwinPointer(new Metadata());
         return 0;
     }
 
     // Main class constructor -- takes a file path, loads the metadata from it
     PyErr_Clear();
-    const char* file;
-    if (PyArg_ParseTuple(args, "s", &file)) {
-        App::Metadata a(file);
-        *(getMetadataPtr()) = a;
-        return 0;
+    const char* filename;
+    if (PyArg_ParseTuple(args, "s", &filename)) {
+        try {
+            auto md = new Metadata(filename);
+            setTwinPointer(md);
+            return 0;
+        }
+        catch (...) {
+            PyErr_SetString(Base::BaseExceptionFreeCADError, "Failed to create Metadata object");
+            return -1;
+        }
     }
 
     // Copy constructor
@@ -85,11 +81,11 @@ int MetadataPy::PyInit(PyObject* args, PyObject* /*kwd*/)
     PyObject* o;
     if (PyArg_ParseTuple(args, "O!", &(App::MetadataPy::Type), &o)) {
         App::Metadata* a = static_cast<App::MetadataPy*>(o)->getMetadataPtr();
-        *(getMetadataPtr()) = *a;
+        setTwinPointer(new Metadata(*a));
         return 0;
     }
 
-    PyErr_SetString(Base::BaseExceptionFreeCADError, "path to metadata file expected");
+    PyErr_SetString(Base::BaseExceptionFreeCADError, "metadata object or path to metadata file expected");
     return -1;
 }
 
diff --git a/src/Mod/Test/Metadata.py b/src/Mod/Test/Metadata.py
index 3a31c30ab3..0fef9236e8 100644
--- a/src/Mod/Test/Metadata.py
+++ b/src/Mod/Test/Metadata.py
@@ -55,6 +55,20 @@ class TestMetadata(unittest.TestCase):
         tags = md.Tag
         self.assertEqual(len(tags), 2)
 
+    def test_copy_constructor(self):
+        filename = os.path.join(self.test_dir, "basic_metadata.xml")
+        md = FreeCAD.Metadata(filename)
+        copy_of_md = FreeCAD.Metadata(md)
+        self.assertEqual(md.Name, copy_of_md.Name)
+        self.assertEqual(md.Description, copy_of_md.Description)
+        self.assertEqual(md.Version, copy_of_md.Version)
+
+    def test_default_constructor(self):
+        try:
+            md = FreeCAD.Metadata()
+        except Exception:
+            self.fail("Metadata default constructor failed")
+
     def test_content_types(self):
         pass