-
Notifications
You must be signed in to change notification settings - Fork 3
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
chore: move domain event fanout from app to bus #1406
chore: move domain event fanout from app to bus #1406
Conversation
# Conflicts: # tests/Digdir.Domain.Dialogporten.Application.Unit.Tests/Digdir.Domain.Dialogporten.Application.Unit.Tests.csproj
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.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (21)
src/Digdir.Domain.Dialogporten.Domain/DomainExtensions.cs (1)
7-10
: Consider adding XML documentation.Since this method is part of the public API and plays a crucial role in event handling, consider adding XML documentation to explain its purpose, behavior, and relationship with the event fanout mechanism.
+ /// <summary> + /// Discovers all non-abstract types implementing IDomainEvent that are defined in the domain assembly. + /// Used for event handling and fanout mechanisms. + /// </summary> + /// <returns>A collection of domain event types.</returns> public static IEnumerable<Type> GetDomainEventTypes() => DomainAssemblyMarker.Assembly.DefinedTypes .Where(x => !x.IsAbstract && !x.IsInterface && !x.IsGenericType) .Where(x => x.IsAssignableTo(typeof(IDomainEvent)));src/Digdir.Domain.Dialogporten.Service/Common/MassTransitApplicationUtils.cs (2)
8-19
: Consider architectural implications and add documentation.While the implementation is clean, this utility creates a tight coupling between the service and application layers, which was noted as a concern in the PR objectives. This coupling might make it harder to maintain layer independence and could complicate future architectural changes.
Consider:
- Introducing an abstraction layer or interface to reduce direct coupling
- Moving this logic to a separate integration layer
- Adding XML documentation to explain the purpose and architectural implications
Add XML documentation to explain:
- The purpose of this method
- The relationship between application handlers and bus consumers
- Any assumptions about the handler and event types
+/// <summary> +/// Maps application-level event handlers to their corresponding MassTransit consumer types. +/// This mapping enables the automatic registration of domain event consumers in the service bus. +/// </summary> +/// <returns>An array of mappings between application handlers and their bus consumer types.</returns> public static ApplicationConsumerMapping[] GetApplicationConsumerMaps()
23-23
: Add documentation for the mapping record.The record struct is well-designed, but would benefit from documentation explaining its purpose and the relationship between its components.
Add XML documentation:
+/// <summary> +/// Represents a mapping between an application-level event handler and its corresponding bus consumer types. +/// </summary> +/// <param name="AppConsumerType">The application-level event handler type</param> +/// <param name="BusConsumerType">The MassTransit consumer type for handling the event</param> +/// <param name="BusDefinitionType">The MassTransit consumer definition type for configuration</param> internal record struct ApplicationConsumerMapping(Type AppConsumerType, Type BusConsumerType, Type BusDefinitionType);src/Digdir.Domain.Dialogporten.Application/Common/ApplicationEventHandlerUtils.cs (1)
6-24
: Consider architectural implications of this utility class.This utility class, while functional, might increase coupling between service and application layers as mentioned in the PR objectives. Consider:
- Whether this mapping logic should live in the bus layer instead
- If we need an abstraction layer to maintain the application's independence
- How this affects other presentation layers that need to publish events via
MediatR.Publish(event)
tests/Digdir.Domain.Dialogporten.Application.Unit.Tests/Digdir.Domain.Dialogporten.Application.Unit.Tests.csproj (1)
15-15
: LGTM! Good choice adding Verify.Xunit for snapshot testing.The addition of Verify.Xunit is well-aligned with the PR's goal of validating event handler mappings and endpoint names. This will help ensure the correctness of the event fanout mechanism as it moves to the bus.
Consider adding a comment in the test project's README or documentation about the snapshot testing setup and conventions to help other developers understand how to work with the generated snapshots.
src/Digdir.Domain.Dialogporten.Application/Common/EndpointNameAttribute.cs (4)
7-18
: Consider enhancing the documentation with examples.While the documentation is clear, adding code examples would make it more developer-friendly.
Consider adding an example like this to the remarks:
/// <remarks> /// <list type="bullet"> <item>Will default to "{handlerType.Name}_{eventType.Name}" if not specified.</item> <item>MassTransit will only wrap <see cref="INotificationHandler{TNotification}"/> where TNotification implements <see cref="IDomainEvent"/>.</item> + <item>Example usage: + [EndpointName("CustomEndpoint")] + public Task Handle(MyEvent notification, CancellationToken cancellationToken) + </item> /// </list> /// </remarks>
19-29
: Consider adding length and character validation for endpoint names.While the basic null/whitespace validation is good, endpoint names might have additional constraints from MassTransit or Azure Service Bus.
Consider adding more specific validation:
public EndpointNameAttribute(string name) { if (string.IsNullOrWhiteSpace(name)) { throw new ArgumentException("Value cannot be null or whitespace.", nameof(name)); } + if (name.Length > 50) // Adjust limit based on actual constraints + { + throw new ArgumentException("Name exceeds maximum length.", nameof(name)); + } + if (!System.Text.RegularExpressions.Regex.IsMatch(name, "^[a-zA-Z0-9-_]+$")) + { + throw new ArgumentException("Name contains invalid characters.", nameof(name)); + } Name = name; }
43-45
: Consider sanitizing type names in the default naming convention.The current implementation might produce invalid endpoint names if type names contain special characters or are too long.
Consider adding name sanitization:
private static string DefaultName(Type handlerType, Type eventType) - => $"{handlerType.Name}_{eventType.Name}"; +{ + var sanitizedHandler = SanitizeTypeName(handlerType.Name); + var sanitizedEvent = SanitizeTypeName(eventType.Name); + return $"{sanitizedHandler}_{sanitizedEvent}"; +} + +private static string SanitizeTypeName(string name) +{ + // Remove generic markers and invalid characters + var sanitized = System.Text.RegularExpressions.Regex.Replace(name, @"`\d+", string.Empty); + sanitized = System.Text.RegularExpressions.Regex.Replace(sanitized, @"[^a-zA-Z0-9-_]", "_"); + return sanitized.Length > 50 ? sanitized[..50] : sanitized; +}
1-45
: Consider architectural implications of this implementation.While the implementation is solid, placing this attribute in the Application layer raises some architectural concerns:
- It tightly couples the Application layer to MassTransit implementation details
- It might complicate using alternative message bus implementations
- As per PR objectives, this change increases coupling between service and application layers
Consider:
- Moving this to the Infrastructure/Service layer
- Creating an abstraction in the Application layer that the Infrastructure layer implements
src/Digdir.Domain.Dialogporten.Application/Features/V1/Common/Events/DialogActivityEventToAltinnForwarder.cs (2)
Line range hint
33-52
: Consider using constants for dictionary keys.To improve maintainability and prevent typos, consider extracting the dictionary keys into constants.
+private static class EventDataKeys +{ + public const string ActivityId = "activityId"; + public const string ExtendedActivityType = "extendedActivityType"; + public const string Process = "process"; + public const string PrecedingProcess = "precedingProcess"; +} private static Dictionary<string, object> GetCloudEventData(DialogActivityCreatedDomainEvent domainEvent) { var data = new Dictionary<string, object> { - ["activityId"] = domainEvent.ActivityId.ToString() + [EventDataKeys.ActivityId] = domainEvent.ActivityId.ToString() }; if (domainEvent.ExtendedType is not null) { - data["extendedActivityType"] = domainEvent.ExtendedType.ToString(); + data[EventDataKeys.ExtendedActivityType] = domainEvent.ExtendedType.ToString(); } if (domainEvent.Process is not null) { - data["process"] = domainEvent.Process; + data[EventDataKeys.Process] = domainEvent.Process; } if (domainEvent.PrecedingProcess is not null) { - data["precedingProcess"] = domainEvent.PrecedingProcess; + data[EventDataKeys.PrecedingProcess] = domainEvent.PrecedingProcess; } return data; }
Line range hint
1-52
: Consider architectural implications of bypassing MediatR.As mentioned in the PR objectives, this implementation bypasses MediatR as the primary entry point and increases coupling between service and application layers. Consider:
- Documenting this architectural decision and its trade-offs
- Evaluating if other presentation layers need to publish events via MediatR
- Assessing the impact on system maintainability and testability
src/Digdir.Domain.Dialogporten.Service/Program.cs (1)
61-64
: Consider architectural implications of the new event handling approach.While the new implementation successfully moves domain event fanout to the bus, there are some architectural considerations:
- This change increases coupling between service and application layers, as noted in the PR objectives.
- The deviation from MediatR as the primary entry point might impact maintainability and consistency.
- Other presentation layers might need to publish events via
MediatR.Publish(event)
.Consider:
- Documenting the architectural decision and its trade-offs
- Evaluating if a facade pattern could help maintain loose coupling
- Ensuring other presentation layers can still use MediatR for event publishing
src/Digdir.Domain.Dialogporten.Application/Features/V1/Common/Events/DialogEventToAltinnForwarder.cs (2)
Line range hint
18-89
: Consider reducing code duplication in event handlersThe four Handle methods share nearly identical code structure, differing only in the event type. This violates the DRY principle.
Consider extracting the common logic into a generic method:
+private async Task HandleEvent<T>(T domainEvent, CancellationToken cancellationToken) where T : IDomainEvent, IProcessEvent +{ + var cloudEvent = new CloudEvent + { + Id = domainEvent.EventId, + Type = CloudEventTypes.Get(nameof(T)), + Time = domainEvent.OccuredAt, + Resource = domainEvent.ServiceResource, + ResourceInstance = domainEvent.DialogId.ToString(), + Subject = domainEvent.Party, + Source = $"{SourceBaseUrl()}{domainEvent.DialogId}", + Data = GetCloudEventData(domainEvent) + }; + await CloudEventBus.Publish(cloudEvent, cancellationToken); +} [EndpointName("DialogEventToAltinnForwarder_DialogCreatedDomainEvent")] -public async Task Handle(DialogCreatedDomainEvent domainEvent, CancellationToken cancellationToken) +public Task Handle(DialogCreatedDomainEvent domainEvent, CancellationToken cancellationToken) { - var cloudEvent = new CloudEvent - { - Id = domainEvent.EventId, - Type = CloudEventTypes.Get(nameof(DialogCreatedDomainEvent)), - Time = domainEvent.OccuredAt, - Resource = domainEvent.ServiceResource, - ResourceInstance = domainEvent.DialogId.ToString(), - Subject = domainEvent.Party, - Source = $"{SourceBaseUrl()}{domainEvent.DialogId}", - Data = GetCloudEventData(domainEvent) - }; - await CloudEventBus.Publish(cloudEvent, cancellationToken); + return HandleEvent(domainEvent, cancellationToken); }
Line range hint
91-105
: Consider using strongly-typed data structureThe GetCloudEventData method uses Dictionary<string, object> which loses type safety. Consider creating a dedicated type for the cloud event data.
+public record CloudEventProcessData +{ + public Process? Process { get; init; } + public Process? PrecedingProcess { get; init; } +} -private static Dictionary<string, object>? GetCloudEventData(IProcessEvent domainEvent) +private static CloudEventProcessData? GetCloudEventData(IProcessEvent domainEvent) { - var data = new Dictionary<string, object>(); - if (domainEvent.Process is not null) - { - data["process"] = domainEvent.Process; - } - if (domainEvent.PrecedingProcess is not null) - { - data["precedingProcess"] = domainEvent.PrecedingProcess; - } - return data.Count == 0 ? null : data; + if (domainEvent.Process is null && domainEvent.PrecedingProcess is null) + return null; + + return new CloudEventProcessData + { + Process = domainEvent.Process, + PrecedingProcess = domainEvent.PrecedingProcess + }; }tests/Digdir.Domain.Dialogporten.Application.Unit.Tests/Features/V1/Common/Utils/ApplicationEventHandlerUtilsTests.cs (2)
8-24
: Well-designed test for preventing message routing conflicts!This test is particularly important given the architectural change to move domain event fanout to the bus. It ensures that each handler has a unique endpoint name, preventing message routing conflicts in Azure Service Bus queues.
Consider adding a test to verify that the
EndpointNameAttribute
correctly overrides the default endpoint name when specified, as this is a key feature mentioned in the PR objectives for handling renamed events/handlers.
69-107
: Robust test for event type stability with thorough documentation!The test effectively tracks event type changes and the documentation excellently covers schema compatibility concerns and dead-letter queue handling.
Consider enhancing the test to also verify schema versioning:
public async Task Developer_Should_Use_Caution_When_Modifying_Events() { // Act var map = ApplicationEventHandlerUtils .GetHandlerEventMaps() - .Select(x => x.EventType.FullName) + .Select(x => new + { + EventType = x.EventType.FullName, + SchemaVersion = x.EventType.GetCustomAttribute<SchemaVersionAttribute>()?.Version + }) .Order() .Distinct() .ToArray(); // Assert await Verify(map); }This would help track both event type and schema version changes, providing an extra layer of compatibility checking.
src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/IdempotentNotifications/INotificationProcessingContextFactory.cs (3)
66-82
: Consider adding debug logging before throwing UnreachableException.The implementation is robust and thread-safe, with excellent comments explaining the rationale. However, it would be helpful to log diagnostic information if we somehow hit the "unreachable" case.
- : throw new UnreachableException("The context should be alive at this point in time."); + : throw new UnreachableException("The context should be alive at this point in time. " + + $"EventId: {eventId}, Context: {context}, WeakContext: {weakContext}");
Line range hint
89-109
: Enhance warning log message with more specific details.The error handling is good, but the warning message could be more specific about the potential impact and include context about what was being processed.
- _logger.LogWarning(e, "An unhandled exception occurred in the notification processing context cleanup task. This may lead to memory leaks."); + _logger.LogWarning(e, "Failed to clean up expired notification processing contexts. This may lead to memory leaks if contexts are not being properly disposed. Dictionary size: {ContextCount}", _contextByEventId.Count);
113-121
: Consider adding readonly modifier for consistency.The method is well-implemented using ValueTask for efficiency. Consider adding the readonly modifier to align with C# best practices for struct methods that don't modify state.
- private async ValueTask<bool> WaitForNextTickSafeAsync() + private readonly async ValueTask<bool> WaitForNextTickSafeAsync()src/Digdir.Domain.Dialogporten.Service/Consumers/DomainEventConsumer.cs (2)
9-13
: Be cautious of increased coupling between the service and application layersThe direct invocation of
_handler.Handle
inDomainEventConsumer<THandler, TEvent>
introduces tighter coupling between the service layer and application handlers. This design deviates from the application architecture that positions MediatR as the primary entry point for handling events. Such coupling can make the system less modular and harder to maintain, especially if other presentation layers need to interact with the application viaMediatR.Publish(event)
.Consider maintaining the abstraction provided by
IPublisher
or using a mediator pattern to decouple the service and application layers. This approach would align with the original architecture and facilitate easier integration with other components.
Line range hint
17-38
: Re-evaluate the direct configuration of consumers for better adherence to architectural principlesThe
DomainEventConsumerDefinition<THandler, TEvent>
directly configures the consumer with specific handlers and events, which can further entrench the coupling between layers. This tight coupling may make future changes more complex and reduce the flexibility of the system.To promote better modularity and adherence to the SOLID principles, consider introducing an abstraction layer or leveraging dependency injection to manage consumer configurations. This adjustment can help in isolating the service layer from the application layer, making the codebase more maintainable and scalable.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (13)
src/Digdir.Domain.Dialogporten.Application/Common/ApplicationEventHandlerUtils.cs
(1 hunks)src/Digdir.Domain.Dialogporten.Application/Common/EndpointNameAttribute.cs
(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/Common/Events/DialogActivityEventToAltinnForwarder.cs
(2 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/Common/Events/DialogEventToAltinnForwarder.cs
(5 hunks)src/Digdir.Domain.Dialogporten.Domain/DomainExtensions.cs
(1 hunks)src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/IdempotentNotifications/INotificationProcessingContextFactory.cs
(3 hunks)src/Digdir.Domain.Dialogporten.Service/Common/MassTransitApplicationUtils.cs
(1 hunks)src/Digdir.Domain.Dialogporten.Service/Consumers/DomainEventConsumer.cs
(1 hunks)src/Digdir.Domain.Dialogporten.Service/Program.cs
(2 hunks)tests/Digdir.Domain.Dialogporten.Application.Unit.Tests/Digdir.Domain.Dialogporten.Application.Unit.Tests.csproj
(1 hunks)tests/Digdir.Domain.Dialogporten.Application.Unit.Tests/Features/V1/Common/Utils/ApplicationEventHandlerUtilsTests.Developer_Should_Use_Caution_When_Modifying_Endpoints.verified.txt
(1 hunks)tests/Digdir.Domain.Dialogporten.Application.Unit.Tests/Features/V1/Common/Utils/ApplicationEventHandlerUtilsTests.Developer_Should_Use_Caution_When_Modifying_Events.verified.txt
(1 hunks)tests/Digdir.Domain.Dialogporten.Application.Unit.Tests/Features/V1/Common/Utils/ApplicationEventHandlerUtilsTests.cs
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- tests/Digdir.Domain.Dialogporten.Application.Unit.Tests/Features/V1/Common/Utils/ApplicationEventHandlerUtilsTests.Developer_Should_Use_Caution_When_Modifying_Endpoints.verified.txt
- tests/Digdir.Domain.Dialogporten.Application.Unit.Tests/Features/V1/Common/Utils/ApplicationEventHandlerUtilsTests.Developer_Should_Use_Caution_When_Modifying_Events.verified.txt
🧰 Additional context used
📓 Learnings (1)
src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/IdempotentNotifications/INotificationProcessingContextFactory.cs (2)
Learnt from: MagnusSandgren
PR: digdir/dialogporten#1277
File: src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/IdempotentNotifications/INotificationProcessingContextFactory.cs:7-11
Timestamp: 2024-10-17T09:49:19.855Z
Learning: In the `digdir/dialogporten` project (C# codebase using .NET), the `INotificationProcessingContextFactory` interface defined in `src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/IdempotentNotifications/INotificationProcessingContextFactory.cs` must remain `public` because it is used by the Service layer. Therefore, its visibility cannot be changed to `internal`.
Learnt from: MagnusSandgren
PR: digdir/dialogporten#1277
File: src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/IdempotentNotifications/IdempotentNotificationHandler.cs:27-27
Timestamp: 2024-10-17T07:06:56.484Z
Learning: In the `IdempotentNotificationHandler<TNotification>` class within the `digdir/dialogporten` project (C# codebase using .NET), the `INotificationProcessingContextFactory.GetExistingContext` method will never return `null`; it will either return a context or throw an exception if no context is found.
🔇 Additional comments (19)
src/Digdir.Domain.Dialogporten.Domain/DomainExtensions.cs (1)
8-10
: LGTM! Performance improvement in type discovery.
The switch to DefinedTypes
from GetTypes()
is a good optimization. It's more precise as it only loads types defined in the assembly, avoiding unnecessary type loading of referenced assemblies.
src/Digdir.Domain.Dialogporten.Service/Common/MassTransitApplicationUtils.cs (3)
1-4
: LGTM! Clean and focused imports.
The imports and namespace are well-organized and follow the project structure.
6-7
: LGTM! Appropriate class modifiers.
The internal static class is well-suited for assembly-scoped utilities with no state.
8-19
: Verify consumer registration and handler compatibility.
Since this utility is central to the new event fanout mechanism, we should verify its integration points.
✅ Verification successful
Let me gather more information about the event handler setup to complete the verification.
Based on the comprehensive verification results, I can now provide a final response:
The utility implementation is correctly integrated and compatible with existing handlers.
The verification shows:
- The utility is properly integrated in
Program.cs
for consumer registration - The implementation correctly maps domain event handlers to MassTransit consumers:
ApplicationEventHandlerUtils
scans forINotificationHandler<T>
implementationsDomainEventConsumer<THandler, TEvent>
properly wraps these handlers- Existing handlers (DialogEventToAltinnForwarder, DialogActivityEventToAltinnForwarder) are compatible
- The consumer configuration includes proper error handling with delayed redelivery and retry policies
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the integration and impact of the new utility
# Check where this utility is used for consumer registration
echo "Checking usage of GetApplicationConsumerMaps:"
rg "GetApplicationConsumerMaps" -A 5
# Look for existing event handlers that might be affected
echo -e "\nChecking existing event handlers:"
ast-grep --pattern 'class $_ : INotificationHandler<$_>'
# Check for any direct MediatR.Publish usage that might be affected
echo -e "\nChecking direct MediatR.Publish usage:"
rg "Publish\(" -A 2
Length of output: 6843
Script:
#!/bin/bash
# Check for ApplicationEventHandlerUtils usage and implementation
echo "Checking ApplicationEventHandlerUtils implementation:"
rg "ApplicationEventHandlerUtils" -A 10
# Check for DomainEventConsumer implementation
echo -e "\nChecking DomainEventConsumer implementation:"
rg "DomainEventConsumer" -A 10
# Look for any notification handlers in the codebase
echo -e "\nChecking for notification handlers:"
rg "INotificationHandler" -A 5
Length of output: 28181
src/Digdir.Domain.Dialogporten.Application/Common/ApplicationEventHandlerUtils.cs (3)
1-4
: LGTM! Clean and focused imports.
The imports are minimal and relevant to the functionality being implemented.
26-26
: LGTM! Appropriate use of record struct.
The record struct is a good choice for this immutable value type that holds two reference type fields.
12-22
: Add error handling for reflection operations.
The reflection code could throw exceptions if the assembly is not properly loaded or if there are issues with type resolution. Consider adding try-catch blocks and logging.
Let's verify the potential error cases:
src/Digdir.Domain.Dialogporten.Application/Common/EndpointNameAttribute.cs (2)
1-5
: LGTM! Imports and namespace are well-organized.
The using statements are appropriately scoped for the required functionality.
31-34
: LGTM! Well-constrained generic method.
The generic constraints and implementation are clean and type-safe.
src/Digdir.Domain.Dialogporten.Application/Features/V1/Common/Events/DialogActivityEventToAltinnForwarder.cs (3)
Line range hint 1-14
: LGTM! Clean class structure with proper dependency injection.
The class follows SOLID principles with clear responsibilities and proper dependency injection.
15-16
: EndpointName follows the new mapping strategy.
The attribute usage aligns with the PR's objective of maintaining mapping between Azure Service Bus Queue and application handlers.
Line range hint 17-31
: Consider adding error handling and source URL validation.
The Handle method should include error handling for potential failures during cloud event publishing. Additionally, verify that the source URL construction is safe and properly encoded.
public async Task Handle(DialogActivityCreatedDomainEvent domainEvent, CancellationToken cancellationToken)
{
+ try
+ {
var cloudEvent = new CloudEvent
{
Id = domainEvent.EventId,
Type = CloudEventTypes.Get(domainEvent.TypeId.ToString()),
Time = domainEvent.OccuredAt,
Resource = domainEvent.ServiceResource,
ResourceInstance = domainEvent.DialogId.ToString(),
Subject = domainEvent.Party,
- Source = $"{SourceBaseUrl()}{domainEvent.DialogId}/activities/{domainEvent.ActivityId}",
+ Source = Uri.EscapeDataString($"{SourceBaseUrl()}{domainEvent.DialogId}/activities/{domainEvent.ActivityId}"),
Data = GetCloudEventData(domainEvent)
};
await CloudEventBus.Publish(cloudEvent, cancellationToken);
+ }
+ catch (Exception ex)
+ {
+ throw new ApplicationException($"Failed to publish activity event for dialog {domainEvent.DialogId}", ex);
+ }
}
src/Digdir.Domain.Dialogporten.Service/Program.cs (2)
9-9
: LGTM: Import change reflects the new architecture.
The change from Service.Consumers
to Service.Common
aligns with the reorganization of event handling components.
61-64
: Verify the completeness of consumer registration.
The new consumer registration approach using MassTransitApplicationUtils.GetApplicationConsumerMaps()
streamlines the configuration, but we should verify that all existing consumers are properly mapped.
Let's verify the consumer mappings:
✅ Verification successful
Let's gather more information about the consumer mappings and their implementation to ensure completeness.
This will help us understand:
- How the consumer mappings are implemented
- What the DomainEventConsumer handles
- How ApplicationConsumerMapping is used in the codebase
Let's check one final piece - the ApplicationEventHandlerUtils implementation to understand how the handler-event mappings are discovered.
Let's try one more search with a different pattern to find the event handlers:
This should help us find the actual event handler implementations to verify they're properly discovered by the registration mechanism.
Let's try one final search focusing on the specific event handlers we found earlier:
Based on the comprehensive analysis of the codebase, I can now provide the final response:
Consumer registration is properly implemented and complete ✓
The new consumer registration approach using MassTransitApplicationUtils.GetApplicationConsumerMaps()
is well-designed and ensures complete coverage:
- It automatically discovers all
INotificationHandler<T>
implementations throughApplicationEventHandlerUtils.GetHandlerEventMaps()
- The discovered handlers are properly mapped to their corresponding
DomainEventConsumer<THandler, TEvent>
andDomainEventConsumerDefinition<THandler, TEvent>
- All event handlers (DialogEventToAltinnForwarder and DialogActivityEventToAltinnForwarder) are properly decorated with
EndpointName
attributes - The registration includes proper retry policies and error handling through the consumer definitions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Compare old and new consumer registrations to ensure completeness
# Search for old consumer registrations
echo "Old consumer registrations:"
rg -l "DomainEventConsumer[^a-zA-Z]" --type cs
# Search for new consumer mappings
echo "\nNew consumer mappings:"
rg -l "ApplicationConsumerMapping[^a-zA-Z]" --type cs
# Search for any MediatR event handlers that might need to be considered
echo "\nMediatR event handlers:"
rg -l "INotificationHandler<.*>" --type cs
Length of output: 1376
Script:
#!/bin/bash
# Check the implementation of GetApplicationConsumerMaps and related types
echo "=== MassTransitApplicationUtils implementation ==="
rg -A 10 "GetApplicationConsumerMaps" src/Digdir.Domain.Dialogporten.Service/Common/MassTransitApplicationUtils.cs
echo "\n=== DomainEventConsumer implementation ==="
cat src/Digdir.Domain.Dialogporten.Service/Consumers/DomainEventConsumer.cs
echo "\n=== ApplicationConsumerMapping usage ==="
rg "ApplicationConsumerMapping" src/Digdir.Domain.Dialogporten.Service/Common/MassTransitApplicationUtils.cs
Length of output: 3320
Script:
#!/bin/bash
# Check ApplicationEventHandlerUtils implementation
echo "=== ApplicationEventHandlerUtils implementation ==="
cat src/Digdir.Domain.Dialogporten.Application/Common/ApplicationEventHandlerUtils.cs
# Check actual event handlers
echo "\n=== Event Handler implementations ==="
rg -l "class.*:.*INotificationHandler<.*>" --type cs
Length of output: 1484
Script:
#!/bin/bash
# Find event handlers with different patterns
echo "=== Event Handlers (Pattern 1) ==="
rg "class.*implements.*INotificationHandler" --type cs
echo "\n=== Event Handlers (Pattern 2) ==="
rg "class.*:.*INotificationHandler" --type cs
echo "\n=== Event Handlers (Pattern 3) ==="
ast-grep --pattern 'class $_ : INotificationHandler<$_> { $$$ }'
Length of output: 416
Script:
#!/bin/bash
# Check the implementation of specific event handlers we found
echo "=== DialogActivityEventToAltinnForwarder ==="
cat src/Digdir.Domain.Dialogporten.Application/Features/V1/Common/Events/DialogActivityEventToAltinnForwarder.cs
echo "\n=== DialogEventToAltinnForwarder ==="
cat src/Digdir.Domain.Dialogporten.Application/Features/V1/Common/Events/DialogEventToAltinnForwarder.cs
# Check if there are any other event handlers in the Features directory
echo "\n=== Other potential handlers in Features directory ==="
rg -l "Handle.*CancellationToken" src/Digdir.Domain.Dialogporten.Application/Features/
Length of output: 9505
src/Digdir.Domain.Dialogporten.Application/Features/V1/Common/Events/DialogEventToAltinnForwarder.cs (2)
18-18
: Verify endpoint name consistency across the application
The EndpointName attributes follow a pattern of {ClassNameWithoutSuffix}_{EventName}
. Let's ensure this naming convention is consistent.
Also applies to: 35-35, 53-53, 71-71
✅ Verification successful
Let me analyze the results from the rg
command to verify the endpoint naming convention.
Looking at the results, all EndpointName attributes follow the pattern {ClassNameWithoutSuffix}_{EventName}
:
DialogEventToAltinnForwarder_DialogCreatedDomainEvent
DialogEventToAltinnForwarder_DialogUpdatedDomainEvent
DialogEventToAltinnForwarder_DialogSeenDomainEvent
DialogEventToAltinnForwarder_DialogDeletedDomainEvent
DialogEventToAltinnForwarder_DialogActivityCreatedDomainEvent
All endpoint names consistently follow the pattern where:
- First part is the class name without suffix (DialogEventToAltinnForwarder)
- Followed by underscore
- Followed by the event name (e.g., DialogCreatedDomainEvent)
Endpoint naming convention is consistent across the application
The naming pattern {ClassNameWithoutSuffix}_{EventName}
is consistently followed in all EndpointName attributes found in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for all EndpointName attributes to verify naming convention
rg -B 1 "\[EndpointName\(.*\)\]" --type cs
Length of output: 1472
Line range hint 1-16
: Verify the impact of moving event fanout to bus
The class currently implements MediatR's INotificationHandler interfaces, which according to the PR objectives, might conflict with the goal of moving fanout to the bus and reducing dependencies.
Let's check if there are other presentation layers using MediatR.Publish for these events:
Consider:
- If other presentation layers need these events, maintain the MediatR handlers
- If not, consider migrating to direct bus integration to reduce the dependency chain
✅ Verification successful
Keep the MediatR handlers as they are part of the outbox pattern
Based on the search results, the current implementation is correct and should be maintained because:
- The domain events are captured by
ConvertDomainEventsToOutboxMessagesInterceptor
in the infrastructure layer DialogEventToAltinnForwarder
is the only handler for these events and is responsible for converting them to cloud events- The events are part of a proper outbox pattern implementation, where:
- Events are raised in
DialogEntity
- Intercepted and stored in outbox
- Processed by this forwarder to send to external systems
- Events are raised in
The MediatR handlers here are not causing unnecessary fanout but are an integral part of the outbox pattern implementation for reliable message delivery.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for MediatR.Publish usage with dialog events
rg -A 2 "Publish\(<.*Dialog(Created|Updated|Seen|Deleted)DomainEvent" --type cs
Length of output: 81
Script:
#!/bin/bash
# Let's try a broader search for dialog event publishing patterns
rg -A 2 "IMediator.*Publish.*Dialog" --type cs
# Also search for direct usage of these domain events
rg -A 2 "Dialog(Created|Updated|Seen|Deleted)DomainEvent" --type cs
# And check if there are any other event handlers for these events
ast-grep --pattern 'class $_ : $$$, INotificationHandler<$_DialogDomainEvent>'
Length of output: 14971
tests/Digdir.Domain.Dialogporten.Application.Unit.Tests/Features/V1/Common/Utils/ApplicationEventHandlerUtilsTests.cs (2)
1-7
: LGTM!
The class structure and imports are clean and follow best practices.
26-67
: Excellent documentation and test for endpoint stability!
The comprehensive documentation about deployment scenarios and required actions is invaluable for maintaining system stability during changes.
Let's verify the snapshot content to ensure it captures all expected endpoints:
✅ Verification successful
Snapshot test implementation is working as expected
The snapshot test correctly captures all DialogEventToAltinnForwarder endpoints, which matches the system's event forwarding design. The test successfully tracks endpoint names and will catch any unintended modifications through snapshot verification.
Reasoning:
- The snapshot contains a well-defined set of endpoints for Dialog events (Created, Updated, Deleted, Seen, ActivityCreated)
- The naming convention is consistent and follows a clear pattern
- The test will detect any changes to these endpoints, fulfilling its purpose of preventing accidental modifications
- The documentation accurately describes the implications and required actions for endpoint changes
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all handlers with EndpointNameAttribute are included in the snapshot
# Search for classes with EndpointNameAttribute
ast-grep --pattern 'class $_ {
$$$
[EndpointName($_)]
$$$
}'
# Search for the snapshot file to verify its existence
fd -g "*.verified.txt" -p "*ApplicationEventHandlerUtilsTests*"
Length of output: 257
Script:
#!/bin/bash
# Let's try a different approach to find EndpointNameAttribute usage and verify snapshots
# Search for EndpointNameAttribute usage in the codebase
rg "EndpointNameAttribute" -B 2 -A 2
# Find verified snapshots in tests directory with correct path
fd "verified.txt" tests/
Length of output: 5468
Script:
#!/bin/bash
# Let's examine the snapshot content and find handler implementations
# Check the content of the verified snapshots
cat tests/Digdir.Domain.Dialogporten.Application.Unit.Tests/Features/V1/Common/Utils/ApplicationEventHandlerUtilsTests.Developer_Should_Use_Caution_When_Modifying_Endpoints.verified.txt
# Find all notification handler implementations to cross-reference
ast-grep --pattern 'class $_ : INotificationHandler<$_>
{
$$$
}'
Length of output: 549
src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/IdempotentNotifications/INotificationProcessingContextFactory.cs (1)
36-40
: LGTM! Improved error handling and variable naming.
The changes enhance clarity by using more appropriate variable naming ('context' instead of 'transaction') and ensure proper cleanup on initialization failures.
src/Digdir.Domain.Dialogporten.Application/Common/ApplicationEventHandlerUtils.cs
Outdated
Show resolved
Hide resolved
src/Digdir.Domain.Dialogporten.Application/Common/EndpointNameAttribute.cs
Show resolved
Hide resolved
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (12)
src/Digdir.Domain.Dialogporten.Service/Common/MassTransitApplicationUtils.cs (4)
6-7
: Add XML documentation for the utility class.Since this class plays a crucial role in the event handling infrastructure, adding XML documentation would improve maintainability.
+/// <summary> +/// Provides utilities for mapping application event handlers to MassTransit consumers. +/// </summary> internal static class MassTransitApplicationUtils
8-20
: Consider refactoring for better maintainability and error handling.The method could benefit from:
- XML documentation explaining its purpose and return value
- Breaking down the LINQ chain for better readability
- Adding null checks and validation
- Error handling for invalid type configurations
Here's a suggested refactor:
+/// <summary> +/// Gets mappings between application event handlers and their corresponding MassTransit consumers. +/// </summary> +/// <returns>Array of mappings between handlers and consumers.</returns> +/// <exception cref="InvalidOperationException">Thrown when handler mapping is invalid.</exception> public static ApplicationConsumerMapping[] GetApplicationConsumerMaps() { var openDomainEventConsumer = typeof(DomainEventConsumer<,>); var openDomainEventConsumerDefinition = typeof(DomainEventConsumerDefinition<,>); - return ApplicationEventHandlerUtils - .GetHandlerEventMaps() - .Select(x => new ApplicationConsumerMapping( - AppConsumerType: x.HandlerType, - BusConsumerType: openDomainEventConsumer.MakeGenericType(x.HandlerType, x.EventType), - BusDefinitionType: openDomainEventConsumerDefinition.MakeGenericType(x.HandlerType, x.EventType), - EndpointName: x.EndpointName)) - .ToArray(); + + var handlerMaps = ApplicationEventHandlerUtils.GetHandlerEventMaps(); + if (handlerMaps == null || !handlerMaps.Any()) + { + return Array.Empty<ApplicationConsumerMapping>(); + } + + var mappings = new List<ApplicationConsumerMapping>(); + foreach (var map in handlerMaps) + { + try + { + var mapping = new ApplicationConsumerMapping( + AppConsumerType: map.HandlerType, + BusConsumerType: openDomainEventConsumer.MakeGenericType(map.HandlerType, map.EventType), + BusDefinitionType: openDomainEventConsumerDefinition.MakeGenericType(map.HandlerType, map.EventType), + EndpointName: map.EndpointName); + mappings.Add(mapping); + } + catch (ArgumentException ex) + { + throw new InvalidOperationException( + $"Failed to create consumer mapping for handler {map.HandlerType.Name}", ex); + } + } + + return mappings.ToArray();
24-24
: Add XML documentation for the record struct.Adding documentation would help explain the purpose of each property and their relationships.
+/// <summary> +/// Represents a mapping between an application event handler and its corresponding MassTransit consumer types. +/// </summary> +/// <param name="AppConsumerType">The application's event handler type.</param> +/// <param name="BusConsumerType">The MassTransit consumer type that wraps the handler.</param> +/// <param name="BusDefinitionType">The MassTransit consumer definition type.</param> +/// <param name="EndpointName">The endpoint name for the consumer, derived from EndpointNameAttribute.</param> internal record struct ApplicationConsumerMapping(Type AppConsumerType, Type BusConsumerType, Type BusDefinitionType, string EndpointName);
1-24
: Consider architectural implications of increased coupling.While this implementation does increase coupling between service and application layers as noted in the PR objectives, it serves the goal of moving domain event fanout to the bus. However, consider:
- Documenting this architectural decision and its trade-offs
- Ensuring other presentation layers can still publish events via MediatR if needed
- Creating an abstraction layer if this coupling becomes problematic in the future
Would you like assistance in creating an ADR (Architecture Decision Record) to document this change?
src/Digdir.Domain.Dialogporten.Application/Common/ApplicationEventHandlerUtils.cs (3)
8-24
: Add XML documentation for this public API method.Since this is a public API, it should include XML documentation describing its purpose, return value, and any exceptions it might throw.
+/// <summary> +/// Retrieves mappings between domain event handlers and their corresponding events. +/// </summary> +/// <returns>An array of handler-to-event mappings with their endpoint names.</returns> +/// <exception cref="InvalidOperationException">Thrown when the application assembly cannot be located.</exception> public static HandlerEventMapping[] GetHandlerEventMaps()
12-23
: Improve code readability and add defensive programming.The complex LINQ chain could be more readable with intermediate variables, and we should add null checks.
public static HandlerEventMapping[] GetHandlerEventMaps() { var openNotificationHandler = typeof(INotificationHandler<>); var domainEventType = typeof(IDomainEvent); + + var assembly = ApplicationAssemblyMarker.Assembly + ?? throw new InvalidOperationException("Application assembly not found"); + + var eligibleTypes = assembly.DefinedTypes + .Where(x => x is { IsInterface: false, IsAbstract: false }); + + var handlerMappings = eligibleTypes + .SelectMany(type => type .GetInterfaces() .Where(@interface => @interface.IsGenericType && @interface.GetGenericTypeDefinition() == openNotificationHandler && @interface.GetGenericArguments().Single().IsAssignableTo(domainEventType)) .Select(@interface => (@class: type, @event: @interface.GetGenericArguments().Single())) .Select(x => new HandlerEventMapping(x.@class.AsType(), x.@event, EndpointNameAttribute.GetName(x.@class, x.@event)))) .ToArray(); + + return handlerMappings; }
26-26
: Add XML documentation for the HandlerEventMapping record.This public record should include XML documentation for better API documentation.
+/// <summary> +/// Represents a mapping between a domain event handler and its corresponding event type. +/// </summary> +/// <param name="HandlerType">The type of the event handler.</param> +/// <param name="EventType">The type of the domain event.</param> +/// <param name="EndpointName">The name of the endpoint where the handler processes the event.</param> public record struct HandlerEventMapping(Type HandlerType, Type EventType, string EndpointName);src/Digdir.Domain.Dialogporten.Service/Consumers/DomainEventConsumer.cs (3)
7-9
: Well-designed type constraints for event handling.The generic type constraints effectively ensure type safety while moving the event handling responsibility to the bus level. This aligns with the PR objective of isolating event handling logic.
This design:
- Enforces compile-time type safety for event handlers
- Reduces coupling by handling events directly at the bus level
- Provides a clean separation of concerns between event publishing and handling
11-12
: Consider adding domain-specific error handling.While the implementation is clean and focused, consider adding domain-specific error handling or logging before delegating to the handler. This could help with debugging and monitoring specific domain events.
private readonly THandler _handler = handler ?? throw new ArgumentNullException(nameof(handler)); -public Task Consume(ConsumeContext<TEvent> context) => _handler.Handle(context.Message, context.CancellationToken); +public async Task Consume(ConsumeContext<TEvent> context) +{ + try + { + await _handler.Handle(context.Message, context.CancellationToken); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + // Log the specific event type and details for debugging + context.LogError($"Error handling {typeof(TEvent).Name}: {ex.Message}", ex); + throw; + } +}
15-17
: Consider making retry intervals configurable.The retry policy is well-structured with both immediate retries and delayed redelivery. However, these intervals should ideally be configurable through application settings to allow for environment-specific tuning.
Consider injecting configuration:
+public sealed class RetryConfiguration +{ + public TimeSpan[] DelayedRedeliveryIntervals { get; init; } = + { TimeSpan.FromMinutes(1), TimeSpan.FromMinutes(5), + TimeSpan.FromMinutes(10), TimeSpan.FromMinutes(15) }; + public TimeSpan[] ImmediateRetryIntervals { get; init; } = + { TimeSpan.FromMilliseconds(100), TimeSpan.FromMilliseconds(200), + TimeSpan.FromMilliseconds(500), TimeSpan.FromMilliseconds(800), + TimeSpan.FromMilliseconds(1000) }; +}Also applies to: 20-21
src/Digdir.Domain.Dialogporten.Application/Common/EndpointNameAttribute.cs (2)
21-29
: Consider enhancing input validation.While the basic null/whitespace check is good, consider adding:
- Maximum length validation to prevent extremely long endpoint names
- Character validation to ensure compatibility with Azure Service Bus naming rules
public EndpointNameAttribute(string name) { if (string.IsNullOrWhiteSpace(name)) { throw new ArgumentException("Value cannot be null or whitespace.", nameof(name)); } + if (name.Length > 50) // Azure Service Bus queue name limit + { + throw new ArgumentException("Name exceeds maximum length of 50 characters.", nameof(name)); + } + if (!System.Text.RegularExpressions.Regex.IsMatch(name, "^[A-Za-z0-9-_]+$")) + { + throw new ArgumentException("Name can only contain letters, numbers, hyphens, and underscores.", nameof(name)); + } Name = name; }
38-40
: Ensure default names comply with Azure Service Bus constraints.The current implementation might generate names that are too long or contain invalid characters from the type names.
private static string DefaultName(Type handlerType, Type eventType) - => $"{handlerType.Name}_{eventType.Name}"; +{ + var name = $"{handlerType.Name}_{eventType.Name}"; + // Sanitize and truncate if needed + name = new string(name.Select(c => char.IsLetterOrDigit(c) || c == '_' || c == '-' ? c : '_').ToArray()); + return name.Length <= 50 ? name : name[..50]; +}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (5)
src/Digdir.Domain.Dialogporten.Application/Common/ApplicationEventHandlerUtils.cs
(1 hunks)src/Digdir.Domain.Dialogporten.Application/Common/EndpointNameAttribute.cs
(1 hunks)src/Digdir.Domain.Dialogporten.Service/Common/MassTransitApplicationUtils.cs
(1 hunks)src/Digdir.Domain.Dialogporten.Service/Consumers/DomainEventConsumer.cs
(1 hunks)src/Digdir.Domain.Dialogporten.Service/Program.cs
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Digdir.Domain.Dialogporten.Service/Program.cs
🔇 Additional comments (8)
src/Digdir.Domain.Dialogporten.Service/Common/MassTransitApplicationUtils.cs (2)
1-4
: LGTM: Clean namespace organization.
The namespace organization and imports are well-structured and minimal.
8-20
: Verify EndpointName attribute usage.
Let's verify that all mapped handlers have the required EndpointName attribute to prevent runtime issues.
src/Digdir.Domain.Dialogporten.Application/Common/ApplicationEventHandlerUtils.cs (3)
1-7
: LGTM! Clean and well-organized class structure.
The imports are minimal, and the static utility class follows C# conventions.
8-24
: Previous performance concern is still valid.
The previous review comment about caching the handler mappings is still applicable. The reflection-heavy operation should be cached since handler mappings are unlikely to change during runtime.
1-26
: Verify architectural impact of this implementation.
Based on the PR objectives, this implementation raises some architectural concerns:
- Moving event fanout to the bus might bypass MediatR as the primary entry point
- The coupling between service and application layers might increase
Consider:
- How will other presentation layers publish events via
MediatR.Publish(event)
? - Should we maintain parallel paths for both direct MediatR publishing and bus-based fanout?
Let's verify the impact on MediatR usage:
✅ Verification successful
The architectural implementation is well-aligned with the PR objectives
The codebase shows a robust event handling architecture that:
-
Maintains MediatR as the primary entry point while properly separating concerns:
- Domain events implement
IDomainEvent
which extendsINotification
- Application layer handles event mapping via
ApplicationEventHandlerUtils
- Infrastructure layer uses MassTransit with outbox pattern for reliable event delivery
- Domain events implement
-
Successfully moves fanout to the message bus:
- Uses MassTransit's EntityFramework outbox integration for reliable event publishing
- Events are intercepted and converted to outbox messages via
ConvertDomainEventsToOutboxMessagesInterceptor
- Separate service layer consumes events via
DomainEventConsumer
The implementation achieves the architectural goals while maintaining clean separation between layers and ensuring reliable event delivery.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for MediatR.Publish usage in presentation layers
rg -t cs "IMediator.*Publish.*IDomainEvent" --glob '!**/obj/**' --glob '!**/bin/**'
# Search for potential coupling between service and application layers
rg -t cs "namespace.*\.Service.*using.*\.Application" --glob '!**/obj/**' --glob '!**/bin/**'
Length of output: 181
Script:
#!/bin/bash
# Let's broaden the search for MediatR usage patterns
rg -t cs "IMediator|MediatR" -A 3 --glob '!**/obj/**' --glob '!**/bin/**'
# Search for event publishing patterns
rg -t cs "Publish\(<.*Event" -A 2 --glob '!**/obj/**' --glob '!**/bin/**'
# Look for service layer dependencies
fd -e cs -E "obj/*" -E "bin/*" | xargs grep -l "namespace.*\.Service"
# Check for application layer usage
fd -e cs -E "obj/*" -E "bin/*" | xargs grep -l "using.*\.Application"
Length of output: 73196
Script:
#!/bin/bash
# Let's check how events are published and handled
ast-grep --pattern 'class $_ : INotificationHandler<$_>'
# Check for event bus/message broker integration
rg -t cs "MassTransit|IPublishEndpoint" -A 3
# Look for domain event implementations
ast-grep --pattern 'class $_ : IDomainEvent'
Length of output: 57494
src/Digdir.Domain.Dialogporten.Application/Common/EndpointNameAttribute.cs (3)
1-16
: LGTM! Well-documented attribute with clear purpose.
The documentation effectively communicates the attribute's purpose, default behavior, and constraints.
17-20
: LGTM! Well-designed attribute class.
The class is correctly sealed and internal, with an immutable Name property.
31-36
: Performance and reliability concerns in reflection usage.
The previous review comment about caching reflection results and handling edge cases is still valid and should be addressed.
Quality Gate failedFailed conditions See analysis details on SonarCloud Catch issues before they fail your Quality Gate with our IDE extension SonarLint |
Pro:
EndpointNameAttribute
(annet navn kanskje?SubscriptionNameAttribute
?) gjør at vi kan rename event og application handler uten at det knekker mappingen mellom Azure Service Bus Queue og application handler.Cons:
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests
Chores