-
Notifications
You must be signed in to change notification settings - Fork 0
Fix message ordering and improve dependency injection #30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the SparklerNet library to use dependency injection for better architecture, improves message ordering with async/await patterns, and adds cache management functionality. The changes include migrating from IMemoryCache to HybridCache, implementing GetBdSeq metric retrieval, and simplifying the rebirth request callback by removing the device ID parameter.
Key Changes
- Refactored service constructors to accept
IMessageOrderingServiceandIStatusTrackingServicevia dependency injection instead of creating instances internally - Migrated from
IMemoryCachetoHybridCachefor distributed caching support and converted synchronous cache operations to asynchronous - Added
ClearCacheAsync()methods to both services for proper cleanup and implemented global cache tags for efficient bulk operations
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| SparkplugHostApplication.cs | Refactored to use injected services; updated message handling to process DBIRTH/DDEATH through ordering service; added cache clearing on shutdown |
| StatusTrackingService.cs | Added ClearCacheAsync method and global status tag; moved semaphore wait before try block for better resource management |
| MessageOrderingService.cs | Migrated from IMemoryCache to HybridCache; converted all methods to async; removed device ID parameter from methods as ordering is now EdgeNode-level only |
| IStatusTrackingService.cs | Added ClearCacheAsync method to interface |
| IMessageOrderingService.cs | Updated methods to async and removed device ID parameter from RebirthRequestCallback delegate |
| Payload.cs | Implemented GetBdSeq() method to extract bdSeq metric from Birth/Death messages |
| SparkplugMessageEventArgs.cs | Removed MqttApplicationMessageReceivedEventArgs parameter and EventArgs property to simplify the event args |
| MessageOrderingServiceTests.cs | Comprehensive test refactoring to async patterns; added new test cases for concurrent processing, cache clearing, and timer management |
| StatusTrackingServiceTests.cs | Added test for ClearCacheAsync functionality |
| SimpleHostApplication.cs | Updated constructor to accept SparkplugHostApplication via DI instead of creating it |
| Program.cs | Added DI container setup with service registrations for proper dependency resolution |
| .gitignore | Added .trae/ IDE folder to ignore list |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
SparklerNet/HostApplication/Caches/MessageOrderingService.cs:347
- The timer reset logic uses a lambda that disposes the existing timer and creates a new one. However, if multiple timers are being reset rapidly, there's a potential for timer disposal to happen during an active callback execution. Consider checking if the timer is actively executing before disposal or using a more robust timer management pattern.
_reorderTimers.AddOrUpdate(timerKey,
_ => new Timer(OnReorderTimeout, timerKey, _options.SeqReorderTimeout, Timeout.Infinite),
(_, existingTimer) =>
{
existingTimer.Change(_options.SeqReorderTimeout, Timeout.Infinite);
return existingTimer;
});
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This PR includes: 1) Refactored service constructors to use IServiceProvider for dependency resolution 2) Added ClearCacheAsync functionality with global cache tags 3) Implemented GetBdSeq method for bdSeq metric retrieval 4) Improved design using interface-based injection 5) Fixed test ambiguities and optimized code