Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 20, 2025

Proposed changes

CoreNodeManager now inherits from CustomNodeManager2, consolidating the server library's NodeManager implementations into a single architecture. The original CoreNodeManager implementation had no test coverage and used a different internal structure (NodeTable/ILocalNode vs NodeState). An interface-based approach has been introduced to provide better abstraction and flexibility.

Key changes:

  • Created ICoreNodeManager interface that extends INodeManager2 and defines the contract for core node managers
  • Created CoreNodeManager2 inheriting from CustomNodeManager2 and implementing ICoreNodeManager
  • Enables SamplingGroups by default as required
  • Manages namespace 0 (UA standard) and namespace 1 (dynamic server namespace)
  • Provides compatibility layer: DataLock, ImportNodes, AttachNode, CreateUniqueNodeId, DynamicNamespaceIndex
  • Marked original CoreNodeManager as [Obsolete] but kept functional for backward compatibility (now also implements ICoreNodeManager)
  • Updated MasterNodeManager, IServerInternal, ServerInternalData to depend on ICoreNodeManager interface instead of concrete types

Test coverage:

  • Added 7 tests covering instantiation, inheritance, compatibility APIs, node management, and namespace handling
  • All existing tests pass (174 server tests)

Types of changes

  • Enhancement (non-breaking change which adds functionality)
  • Test enhancement (non-breaking change to increase test coverage)

Checklist

  • I have read the CONTRIBUTING doc.
  • I have signed the CLA.
  • I ran tests locally with my changes, all passed.
  • I fixed all failing tests in the CI pipelines.
  • I fixed all introduced issues with CodeQL and LGTM.
  • I have added tests that prove my fix is effective or that my feature works and increased code coverage.
  • I have added necessary documentation (if appropriate).
  • Any dependent changes have been merged and published in downstream modules.

Further comments

The refactoring maintains full backward compatibility by keeping the old CoreNodeManager as obsolete. The new implementation uses the same NodeState-based architecture as CustomNodeManager2, making the codebase more maintainable. SamplingGroups support is now enabled in the core node manager as specified in the requirements.

The introduction of ICoreNodeManager interface provides better separation of concerns and allows the server infrastructure to depend on abstractions rather than concrete implementations, following SOLID principles. Both the deprecated CoreNodeManager and the new CoreNodeManager2 implement this interface, ensuring a smooth transition path.

Original prompt

This section details on the original issue you should resolve

<issue_title>Make CoreNodeManager Inherit from CustomNodeManager2</issue_title>
<issue_description>CoreNodeManager should be based of CustomNodeManager2, so the Server Library only includes one type of NodeManager. Also the current CoreNodeManager is not covered by Tests.

As CustomNodeManager2 has support for SamplingGroups starting with 1.5.377 all important features should be straightforward to migrate.

Considerations when updating:

  • ensure for each method exposed by the NodeManager that all functionality that is not implemented by CustomNodeManager2 that the method is overridden and the necessary functionality is implemented.
  • enable SamplingGroups in the CustomNodeManager2</issue_description>

<agent_instructions>Please reimplement CoreNodeManager based on the CustomNodeManager2 Class.
Leave the existing implmentation in the server library marked as deprecated and ensure it is not used any more.</agent_instructions>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copilot AI changed the title [WIP] Refactor CoreNodeManager to inherit from CustomNodeManager2 Refactor CoreNodeManager to inherit from CustomNodeManager2 Dec 20, 2025
Copilot AI requested a review from romanett December 20, 2025 15:46
@romanett
Copy link
Contributor

@copilot please add an Interface ICoreNodeManager and make the Server depend on that. Make shure both classes implement this interface, also ensure to include INodeManager2 interface in the ICoreNodeManager interface

Copy link
Contributor Author

Copilot AI commented Dec 20, 2025

@copilot please add an Interface ICoreNodeManager and make the Server depend on that. Make shure both classes implement this interface, also ensure to include INodeManager2 interface in the ICoreNodeM...

Done! Created ICoreNodeManager interface that extends INodeManager2 and includes the core node manager-specific methods (DataLock, ImportNodes, CreateUniqueNodeId, DynamicNamespaceIndex). Both CoreNodeManager and CoreNodeManager2 now implement this interface. Updated IServerInternal, ServerInternalData, and MasterNodeManager to depend on the interface instead of the concrete type. All 174 server tests pass. Changes in commit 4fc1afa.

Copilot AI changed the title Refactor CoreNodeManager to inherit from CustomNodeManager2 Refactor CoreNodeManager to inherit from CustomNodeManager2 with ICoreNodeManager interface Dec 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make CoreNodeManager Inherit from CustomNodeManager2

3 participants