Add support to scheduler message #3487
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
lillo42
commented
Jan 20, 2025
•
lillo42
commented
- Remove unnecessary code
- Fixes build
- Add Unit test
- Finish implementation
- Add Sample
- Implement support to HangFire
- Implement support to Quartz
- Implement support to AWS Scheduler
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
| @@ -37,6 +37,8 @@ THE SOFTWARE. */ | |||
| using Paramore.Brighter.FeatureSwitch; | |||
| using Paramore.Brighter.Logging; | |||
| using Paramore.Brighter.Observability; | |||
| using Paramore.Brighter.Scheduler.Events; | |||
There was a problem hiding this comment.
❌ Getting worse: Code Duplication
introduced similar code in: SchedulerAsync,SchedulerAsync,SchedulerPost,SchedulerPost
| @@ -37,6 +37,8 @@ THE SOFTWARE. */ | |||
| using Paramore.Brighter.FeatureSwitch; | |||
| using Paramore.Brighter.Logging; | |||
| using Paramore.Brighter.Observability; | |||
| using Paramore.Brighter.Scheduler.Events; | |||
There was a problem hiding this comment.
ℹ Getting worse: Missing Arguments Abstractions
The average number of function arguments increases from 4.12 to 4.17, threshold = 4.00
Comment on lines
+274
to
+296
| public async Task SchedulerAsync<TRequest>(TimeSpan delay, | ||
| TRequest request, | ||
| RequestContext? requestContext = null, | ||
| bool continueOnCapturedContext = true, | ||
| CancellationToken cancellationToken = default) | ||
| where TRequest : class, IRequest | ||
| { | ||
| if (_messageSchedulerFactory == null) | ||
| { | ||
| throw new InvalidOperationException("No message scheduler factory set."); | ||
| } | ||
|
|
||
| s_logger.LogInformation("Scheduling request: {RequestType} {Id}", request.GetType(), request.Id); | ||
| var scheduler = _messageSchedulerFactory.Create(this); | ||
| if (scheduler is IAmAMessageSchedulerAsync asyncScheduler) | ||
| { | ||
| await asyncScheduler.ScheduleAsync(delay, SchedulerFireType.Post, request, cancellationToken); | ||
| } | ||
| else if (scheduler is IAmAMessageSchedulerSync sync) | ||
| { | ||
| sync.Schedule(delay, SchedulerFireType.Post, request); | ||
| } | ||
| } |
There was a problem hiding this comment.
ℹ New issue: Excess Number of Function Arguments
SchedulerAsync has 5 arguments, threshold = 4
Comment on lines
+299
to
+321
| public async Task SchedulerAsync<TRequest>(DateTimeOffset at, | ||
| TRequest request, | ||
| RequestContext? requestContext = null, | ||
| bool continueOnCapturedContext = true, | ||
| CancellationToken cancellationToken = default) | ||
| where TRequest : class, IRequest | ||
| { | ||
| if (_messageSchedulerFactory == null) | ||
| { | ||
| throw new InvalidOperationException("No message scheduler factory set."); | ||
| } | ||
|
|
||
| s_logger.LogInformation("Scheduling request: {RequestType} {Id}", request.GetType(), request.Id); | ||
| var scheduler = _messageSchedulerFactory.Create(this); | ||
| if (scheduler is IAmAMessageSchedulerAsync asyncScheduler) | ||
| { | ||
| await asyncScheduler.ScheduleAsync(at, SchedulerFireType.Post, request, cancellationToken); | ||
| } | ||
| else if (scheduler is IAmAMessageSchedulerSync sync) | ||
| { | ||
| sync.Schedule(at, SchedulerFireType.Post, request); | ||
| } | ||
| } |
There was a problem hiding this comment.
ℹ New issue: Excess Number of Function Arguments
SchedulerAsync has 5 arguments, threshold = 4
Comment on lines
+132
to
+133
| InstrumentationOptions instrumentationOptions = InstrumentationOptions.All, | ||
| IAmAMessageSchedulerFactory? messageSchedulerFactory = null) |
There was a problem hiding this comment.
ℹ Getting worse: Constructor Over-Injection
CommandProcessor increases from 8 to 9 arguments, threshold = 5
Comment on lines
+183
to
+185
| InstrumentationOptions instrumentationOptions = InstrumentationOptions.All, | ||
| IAmAMessageSchedulerFactory? messageSchedulerFactory = null) | ||
| : this(subscriberRegistry, handlerFactory, requestContextFactory, policyRegistry, featureSwitchRegistry, inboxConfiguration, messageSchedulerFactory: messageSchedulerFactory) |
There was a problem hiding this comment.
ℹ Getting worse: Constructor Over-Injection
CommandProcessor increases from 11 to 12 arguments, threshold = 5
Comment on lines
+95
to
+113
| public void Add(SchedulerMessage message) | ||
| { | ||
| lock (_lock) | ||
| { | ||
| var node = _messages.First; | ||
| while (node != null) | ||
| { | ||
| if (node.Value.At > message.At) | ||
| { | ||
| _messages.AddBefore(node, message); | ||
| return; | ||
| } | ||
|
|
||
| node = node.Next; | ||
| } | ||
|
|
||
| _messages.AddLast(message); | ||
| } | ||
| } |
There was a problem hiding this comment.
❌ New issue: Code Duplication
The module contains 2 functions with similar structure: Add,Delete
| @@ -139,6 +141,7 @@ public Dispatcher Build(string hostName) | |||
| var subscriberRegistry = new SubscriberRegistry(); | |||
| subscriberRegistry.Register<ConfigurationCommand, ConfigurationCommandHandler>(); | |||
| subscriberRegistry.Register<HeartbeatRequest, HeartbeatRequestCommandHandler>(); | |||
| subscriberRegistry.RegisterAsync<SchedulerMessageFired, SchedulerMessageFiredHandlerAsync>(); | |||
There was a problem hiding this comment.
❌ Getting worse: Large Method
Build increases from 73 to 74 lines of code, threshold = 70
Comment on lines
+49
to
+75
| private static ValueTask ExecuteAsync<TRequest>(IAmACommandProcessor commandProcessor, | ||
| string data, | ||
| bool async, | ||
| SchedulerFireType fireType, | ||
| CancellationToken cancellationToken) | ||
| where TRequest : class, IRequest | ||
| { | ||
| var request = JsonSerializer.Deserialize<TRequest>(data, JsonSerialisationOptions.Options)!; | ||
| switch (fireType) | ||
| { | ||
| case SchedulerFireType.Send when async: | ||
| return new ValueTask(commandProcessor.SendAsync(request, cancellationToken: cancellationToken)); | ||
| case SchedulerFireType.Send: | ||
| commandProcessor.Send(request); | ||
| return new ValueTask(); | ||
| case SchedulerFireType.Publish when async: | ||
| return new ValueTask(commandProcessor.PublishAsync(request, cancellationToken: cancellationToken)); | ||
| case SchedulerFireType.Publish: | ||
| commandProcessor.Publish(request); | ||
| return new ValueTask(); | ||
| case SchedulerFireType.Post when async: | ||
| return new ValueTask(commandProcessor.PostAsync(request, cancellationToken: cancellationToken)); | ||
| default: | ||
| commandProcessor.Post(request); | ||
| return new ValueTask(); | ||
| } | ||
| } |
There was a problem hiding this comment.
❌ New issue: Excess Number of Function Arguments
ExecuteAsync has 5 arguments, threshold = 4
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.