Skip to content

Conversation

@longkerdandy
Copy link
Owner

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

Copy link
Contributor

Copilot AI left a 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 IMessageOrderingService and IStatusTrackingService via dependency injection instead of creating instances internally
  • Migrated from IMemoryCache to HybridCache for 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.

Copy link
Contributor

Copilot AI left a 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.

@longkerdandy longkerdandy merged commit a44f853 into develop Dec 4, 2025
10 checks passed
@longkerdandy longkerdandy deleted the feature/fix-message-ordering branch December 4, 2025 03:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants