Correct non-gui DXF C++ importer to not generate pending python exceptions (#20328)

* Add a test case for DXF import

* Test gui flag rather than look for import error to make gui decision

The new code is cleaner and faster and avoids any exception stuff

* Properly avoid trying to use Layer's View object in non-GUI

The code was trying to avoid this but had a Python None object rather than a null C++ pointer and so tried setting a property on None. This left an unhandled exception state which acted as a booby trap that caused the later failure of some unrelated code.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* De-lint, remove wong "unsupported" message
Hidden layers have been supported for a while but still generated an import note about this being unsupported.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

src/Mod/Draft/importDXF.py

@@ -2828,13 +2828,12 @@ def open(filename):
         doc = FreeCAD.newDocument(docname)
         doc.Label = docname
         FreeCAD.setActiveDocument(doc.Name)
-        try:
+        if gui:
             import ImportGui
-        except Exception:
+            ImportGui.readDXF(filename)
+        else:
             import Import
             Import.readDXF(filename)
-        else:
-            ImportGui.readDXF(filename)
         Draft.convert_draft_texts() # convert annotations to Draft texts
         doc.recompute()
 
@@ -2871,13 +2870,12 @@ def insert(filename, docname):
         else:
             errorDXFLib(gui)
     else:
-        try:
+        if gui:
             import ImportGui
-        except Exception:
+            ImportGui.readDXF(filename)
+        else:
             import Import
             Import.readDXF(filename)
-        else:
-            ImportGui.readDXF(filename)
         Draft.convert_draft_texts() # convert annotations to Draft texts
         doc.recompute()
 

src/Mod/Import/App/dxf/ImpExpDxf.cpp

@@ -462,7 +462,7 @@ void ImpExpDxfRead::ExpandInsert(const std::string& name,
                                  double rotation,
                                  const Base::Vector3d& scale)
 {
-    if (Blocks.count(name) == 0) {
+    if (!Blocks.contains(name)) {
         ImportError("Reference to undefined or external block '%s'\n", name);
         return;
     }
@@ -607,15 +607,14 @@ ImpExpDxfRead::Layer::Layer(const std::string& name,
                             std::string&& lineType,
                             PyObject* drawingLayer)
     : CDxfRead::Layer(name, color, std::move(lineType))
-    , DraftLayerView(drawingLayer == nullptr ? nullptr
+    , DraftLayerView(drawingLayer == nullptr ? Py_None
                                              : PyObject_GetAttrString(drawingLayer, "ViewObject"))
-    , GroupContents(
-          drawingLayer == nullptr
-              ? nullptr
-              : (App::PropertyLinkListHidden*)(((App::FeaturePythonPyT<App::DocumentObjectPy>*)
-                                                    drawingLayer)
-                                                   ->getPropertyContainerPtr())
-                    ->getDynamicPropertyByName("Group"))
+    , GroupContents(drawingLayer == nullptr
+                        ? nullptr
+                        : dynamic_cast<App::PropertyLinkListHidden*>(
+                              (((App::FeaturePythonPyT<App::DocumentObjectPy>*)drawingLayer)
+                                   ->getPropertyContainerPtr())
+                                  ->getDynamicPropertyByName("Group")))
 {}
 ImpExpDxfRead::Layer::~Layer()
 {
@@ -631,7 +630,7 @@ void ImpExpDxfRead::Layer::FinishLayer() const
         // App::FeaturePython, and its Proxy is a draftobjects.layer.Layer
         GroupContents->setValue(Contents);
     }
-    if (DraftLayerView != nullptr && Hidden) {
+    if (DraftLayerView != Py_None && Hidden) {
         // Hide the Hidden layers if possible (if GUI exists)
         // We do this now rather than when the layer is created so all objects
         // within the layers also become hidden.
@@ -672,7 +671,7 @@ ImpExpDxfRead::MakeLayer(const std::string& name, ColorIndex_t color, std::strin
                                                          "Solid");
         }
         auto result = new Layer(name, color, std::move(lineType), layer);
-        if (result->DraftLayerView != nullptr) {
+        if (result->DraftLayerView != Py_None) {
             PyObject_SetAttrString(result->DraftLayerView, "OverrideLineColorChildren", Py_False);
             PyObject_SetAttrString(result->DraftLayerView,
                                    "OverrideShapeAppearanceChildren",

src/Mod/Import/App/dxf/ImpExpDxf.h

@@ -147,6 +147,7 @@ protected:
         void operator=(const Layer&) = delete;
         void operator=(Layer&&) = delete;
         ~Layer() override;
+        // The View object for the layer or Py_None (e.g. if no gui)
         PyObject* const DraftLayerView;
         std::vector<App::DocumentObject*> Contents;
         void FinishLayer() const;
@@ -334,7 +335,7 @@ protected:
                        const std::string& name,
                        double rotation) override
         {
-            InsertsList[Reader.m_entityAttributes].push_back(
+            InsertsList[Reader.m_entityAttributes].emplace_back(
                 Block::Insert(name, point, rotation, scale));
         }
 

src/Mod/Import/App/dxf/dxf.cpp

@@ -2950,9 +2950,6 @@ bool CDxfRead::ReadLayer()
         // TODO: Should have an import option to omit frozen layers.
         UnsupportedFeature("Frozen layers");
     }
-    if (layerColor < 0) {
-        UnsupportedFeature("Hidden layers");
-    }
     Layers[layername] = MakeLayer(layername, layerColor, std::move(lineTypeName));
     return true;
 }

src/Mod/Test/CMakeLists.txt

@@ -25,6 +25,7 @@ SET(TestData_SRCS
     TestData/bad_xml.xml
     TestData/bad_version.xml
     TestData/content_items.xml
+    TestData/DXFSample.dxf
 )
 
 SOURCE_GROUP("" FILES ${Test_SRCS} ${TestData_SRCS})

src/Mod/Test/Document.py

@@ -21,6 +21,7 @@
 # *                                                                         *
 # ***************************************************************************/
 
+from plistlib import UID
 import FreeCAD, os, unittest, tempfile
 from FreeCAD import Base
 import math
@@ -671,6 +672,49 @@ class DocumentBasicCases(unittest.TestCase):
         FreeCAD.closeDocument("CreateTest")
 
 
+class DocumentImportCases(unittest.TestCase):
+    def testDXFImportCPPIssue20195(self):
+        import importDXF
+        from draftutils import params
+
+        # Set options, doing our best to restore them:
+        wasShowDialog = params.get_param("dxfShowDialog")
+        wasUseLayers = params.get_param("dxfUseDraftVisGroups")
+        wasUseLegacyImporter = params.get_param("dxfUseLegacyImporter")
+        wasCreatePart = params.get_param("dxfCreatePart")
+        wasCreateDraft = params.get_param("dxfCreateDraft")
+        wasCreateSketch = params.get_param("dxfCreateSketch")
+
+        try:
+            # disable Preferences dialog in gui mode (avoids popup prompt to user)
+            params.set_param("dxfShowDialog", False)
+            # Preserve the DXF layers (makes the checking of document contents easier)
+            params.set_param("dxfUseDraftVisGroups", True)
+            # Use the new C++ importer -- that's where the bug was
+            params.set_param("dxfUseLegacyImporter", False)
+            # create simple part shapes (3 params)
+            # This is required to display the bug because creation of Draft objects clears out the
+            # pending exception this test is looking for, whereas creation of the simple shape object
+            # actually throws on the pending exception so the entity is absent from the document.
+            params.set_param("dxfCreatePart", True)
+            params.set_param("dxfCreateDraft", False)
+            params.set_param("dxfCreateSketch", False)
+            importDXF.insert(
+                FreeCAD.getHomePath() + "Mod/Test/TestData/DXFSample.dxf", "ImportedDocName"
+            )
+        finally:
+            params.set_param("dxfShowDialog", wasShowDialog)
+            params.set_param("dxfUseDraftVisGroups", wasUseLayers)
+            params.set_param("dxfUseLegacyImporter", wasUseLegacyImporter)
+            params.set_param("dxfCreatePart", wasCreatePart)
+            params.set_param("dxfCreateDraft", wasCreateDraft)
+            params.set_param("dxfCreateSketch", wasCreateSketch)
+        doc = FreeCAD.getDocument("ImportedDocName")
+        # This doc should ahve 3 objects: The Layers containter, the DXF layer called 0, and one Line
+        self.assertEqual(len(doc.Objects), 3)
+        FreeCAD.closeDocument("ImportedDocName")
+
+
 # class must be defined in global scope to allow it to be reloaded on document open
 class SaveRestoreSpecialGroup:
     def __init__(self, obj):