Extending API to query required namespaces #348
Conversation
3dJan
requested review from
ewaldbr2,
ewaldbrADSK,
gangatp and
martinweismann
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #348 +/- ##
===========================================
+ Coverage 59.66% 62.82% +3.15%
===========================================
Files 64 65 +1
Lines 24646 26488 +1842
===========================================
+ Hits 14705 16640 +1935
+ Misses 9941 9848 -93 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…mf into QueryRequiredNamespaces
There was a problem hiding this comment.
Pull request overview
This PR extends the lib3mf API to allow clients to query required namespaces for 3MF model extensions. This enables clients to determine model compatibility without parsing XML files or having knowledge of future extensions.
Key Changes:
- Added
GetRequiredNameSpaces()API method to the Model class that returns an iterator over required namespace URIs - Implemented
NameSpaceIteratorclass for iterating through namespace collections - Updated model reader to register required namespaces when parsing 3MF files
Reviewed changes
Copilot reviewed 39 out of 41 changed files in this pull request and generated 19 comments.
Show a summary per file
| File | Description |
|---|---|
Source/Model/Classes/NMR_Model.cpp |
Core implementation of namespace tracking and retrieval logic |
Source/Model/Reader/NMR_ModelReaderNode_ModelBase.cpp |
Registers required namespaces during XML parsing |
Include/Model/Classes/NMR_Model.h |
Added member variable and method declarations |
Source/API/lib3mf_model.cpp |
Wrapper implementation for new API method |
Source/API/lib3mf_namespaceiterator.cpp |
Iterator implementation for namespace collection |
Include/API/lib3mf_namespaceiterator.hpp |
Iterator class definition |
Tests/CPP_Bindings/Source/Model.cpp |
Test coverage for empty, known, and unknown namespaces |
AutomaticComponentToolkit/lib3mf.xml |
API definition for code generation |
CMakeLists.txt |
Added static library build option |
Autogenerated/**/* |
Auto-generated bindings for multiple languages (Python, Pascal, Go, NodeJS, C#, C++, C) |
.gitignore |
Minor updates to ignore patterns |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <<<<<<< HEAD | ||
| pWrapperTable->m_Model_GetRequiredNameSpaces = NULL; | ||
| ======= | ||
| pWrapperTable->m_Model_GetFunctions = NULL; | ||
| pWrapperTable->m_Model_AddImplicitFunction = NULL; | ||
| pWrapperTable->m_Model_AddFunctionFromImage3D = NULL; | ||
| pWrapperTable->m_Model_AddVolumeData = NULL; | ||
| pWrapperTable->m_Model_AddLevelSet = NULL; | ||
| pWrapperTable->m_Model_GetLevelSets = NULL; | ||
| pWrapperTable->m_Model_RemoveResource = NULL; | ||
| >>>>>>> 3732957fa520612a7156de3e01d935ec2569de07 |
There was a problem hiding this comment.
This file contains unresolved merge conflict markers. The conflict appears between the new GetRequiredNameSpaces method and other methods (GetFunctions, AddImplicitFunction, etc.). This must be resolved before merging.
|
|
||
| /** | ||
| * INameSpaceIterator::Count - Returns the number of namespaces the iterator captures. | ||
| * @return returns the number of namspaces the iterator captures. |
There was a problem hiding this comment.
Spelling error: "namspaces" should be "namespaces".
|
|
||
| NameSpaces requiredNameSpaces; | ||
|
|
||
| // Add all namespaces from m_requiredExtensions that are not in knownExtensionNameSpaces to requiredNameSpaces |
There was a problem hiding this comment.
The comment on line 1836 references m_requiredExtensions but the actual variable being iterated is m_requiredNameSpaces. The comment should be updated to match the actual variable name for clarity.
| } | ||
| } // namespace NMR | ||
|
|
||
| void CModel::registerRequiredNameSpace(std::string const&nameSpace) |
There was a problem hiding this comment.
Missing space after const& - should be const& nameSpace for consistency with the coding style used elsewhere in the codebase.
| NameSpaces CModel::getRequiredNameSpaces() | ||
| { | ||
| static const NameSpaces knownExtensionNameSpaces{XML_3MF_NAMESPACE_MATERIALSPEC, | ||
| XML_3MF_NAMESPACE_PRODUCTIONSPEC, | ||
| XML_3MF_NAMESPACE_BEAMLATTICESPEC, | ||
| XML_3MF_NAMESPACE_SLICESPEC, | ||
| XML_3MF_NAMESPACE_SECURECONTENTSPEC, | ||
| XML_3MF_NAMESPACE_DIGITALSIGNATURESPEC, | ||
| XML_3MF_NAMESPACE_CIPHERVALUESPEC}; | ||
|
|
||
| NameSpaces requiredNameSpaces; | ||
|
|
||
| // Add all namespaces from m_requiredExtensions that are not in knownExtensionNameSpaces to requiredNameSpaces | ||
| for (auto const &nameSpace : m_requiredNameSpaces) | ||
| { | ||
| if (std::find(knownExtensionNameSpaces.begin(), knownExtensionNameSpaces.end(), nameSpace) == knownExtensionNameSpaces.end()) | ||
| { | ||
| requiredNameSpaces.push_back(nameSpace); | ||
| } | ||
| } | ||
|
|
||
| // For the namespaces this version of lib3mf knows, we test if the namespace is required by the model | ||
| for (auto const &nameSpace : knownExtensionNameSpaces) | ||
| { | ||
| if (RequireExtension(nameSpace)) | ||
| { | ||
| requiredNameSpaces.push_back(nameSpace); | ||
| } | ||
| } | ||
| return requiredNameSpaces; | ||
| } |
There was a problem hiding this comment.
The getRequiredNameSpaces() method may return duplicate namespaces. If a namespace is both in m_requiredNameSpaces (from unknown extensions) and also happens to be required by the model (via RequireExtension()), it could be added twice to requiredNameSpaces. Consider deduplicating the result or ensuring namespaces aren't added twice.
| public unsafe extern static Int32 KeyStore_SetUUID (IntPtr Handle, byte[] AUUID); | ||
|
|
||
| [DllImport("lib3mf.dll", EntryPoint = "lib3mf_namespaceiterator_movenext", CallingConvention=CallingConvention.Cdecl)] | ||
| public unsafe extern static Int32 NameSpaceIterator_MoveNext (IntPtr Handle, out Byte AHasNext); |
There was a problem hiding this comment.
Minimise the use of unmanaged code.
| public unsafe extern static Int32 NameSpaceIterator_MoveNext (IntPtr Handle, out Byte AHasNext); | |
| public extern static Int32 NameSpaceIterator_MoveNext (IntPtr Handle, out Byte AHasNext); |
| public unsafe extern static Int32 NameSpaceIterator_MoveNext (IntPtr Handle, out Byte AHasNext); | ||
|
|
||
| [DllImport("lib3mf.dll", EntryPoint = "lib3mf_namespaceiterator_moveprevious", CallingConvention=CallingConvention.Cdecl)] | ||
| public unsafe extern static Int32 NameSpaceIterator_MovePrevious (IntPtr Handle, out Byte AHasPrevious); |
There was a problem hiding this comment.
Minimise the use of unmanaged code.
| public unsafe extern static Int32 NameSpaceIterator_MovePrevious (IntPtr Handle, out Byte AHasPrevious); | |
| public extern static Int32 NameSpaceIterator_MovePrevious (IntPtr Handle, out Byte AHasPrevious); |
| public unsafe extern static Int32 NameSpaceIterator_MovePrevious (IntPtr Handle, out Byte AHasPrevious); | ||
|
|
||
| [DllImport("lib3mf.dll", EntryPoint = "lib3mf_namespaceiterator_getcurrent", CallingConvention=CallingConvention.Cdecl)] | ||
| public unsafe extern static Int32 NameSpaceIterator_GetCurrent (IntPtr Handle, UInt32 sizeNameSpace, out UInt32 neededNameSpace, IntPtr dataNameSpace); |
There was a problem hiding this comment.
Minimise the use of unmanaged code.
| public unsafe extern static Int32 NameSpaceIterator_GetCurrent (IntPtr Handle, UInt32 sizeNameSpace, out UInt32 neededNameSpace, IntPtr dataNameSpace); | ||
|
|
||
| [DllImport("lib3mf.dll", EntryPoint = "lib3mf_namespaceiterator_count", CallingConvention=CallingConvention.Cdecl)] | ||
| public unsafe extern static Int32 NameSpaceIterator_Count (IntPtr Handle, out UInt64 ACount); |
There was a problem hiding this comment.
Minimise the use of unmanaged code.
| public unsafe extern static Int32 NameSpaceIterator_Count (IntPtr Handle, out UInt64 ACount); | |
| public extern static Int32 NameSpaceIterator_Count (IntPtr Handle, out UInt64 ACount); |
| public unsafe extern static Int32 Model_RemoveResource (IntPtr Handle, IntPtr AResource); | ||
|
|
||
| [DllImport("lib3mf.dll", EntryPoint = "lib3mf_model_getrequirednamespaces", CallingConvention=CallingConvention.Cdecl)] | ||
| public unsafe extern static Int32 Model_GetRequiredNameSpaces (IntPtr Handle, out IntPtr ANameSpaceIterator); |
There was a problem hiding this comment.
Minimise the use of unmanaged code.
…lib3mf_dynamic.cc] Fix initialization of required namespaces in wrapper table
This PR adds a new API method to the model class that allows to query the namespaces required by extensions. This allows clients to decide weather the model can be loaded or not without any knowledge about future extensions, and without the need to parse the xml-file separately.