-
Notifications
You must be signed in to change notification settings - Fork 5
Validate that EnableCrossEntityTransactions is configured when SendsAtomicWithReceive is enabled #402
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
Validate that EnableCrossEntityTransactions is configured when SendsAtomicWithReceive is enabled #402
Changes from all commits
8f010d1
9b3be63
15f1cee
e731ae8
8c892ff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| using NServiceBus; | ||
|
|
||
| public class SomeEvent : IEvent | ||
| { | ||
| public SomeEvent() | ||
| { | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| using System.Threading.Tasks; | ||
| using NServiceBus; | ||
| using NServiceBus.Logging; | ||
|
|
||
| public class SomeEventMessageHandler : IHandleMessages<SomeEvent> | ||
| { | ||
| static readonly ILog Log = LogManager.GetLogger<SomeEventMessageHandler>(); | ||
|
|
||
| public Task Handle(SomeEvent message, IMessageHandlerContext context) | ||
| { | ||
| Log.Warn($"Handling {nameof(SomeEvent)} in {nameof(SomeEventMessageHandler)}"); | ||
|
|
||
| return Task.CompletedTask; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,8 @@ | ||
| { | ||
| "version": "2.0" | ||
| "version": "2.0", | ||
| "extensions": { | ||
| "ServiceBus": { | ||
| "EnableCrossEntityTransactions": true | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,7 +45,8 @@ public static void UseNServiceBus( | |
| Action<ServiceBusTriggeredEndpointConfiguration> configurationFactory = null) | ||
| { | ||
| var config = functionsHostBuilder.GetContext().Configuration; | ||
| var serviceBusConfiguration = new ServiceBusTriggeredEndpointConfiguration(endpointName, config, null); | ||
|
|
||
| var serviceBusConfiguration = new ServiceBusTriggeredEndpointConfiguration(endpointName, config, GetTransportTransactionMode()); | ||
| configurationFactory?.Invoke(serviceBusConfiguration); | ||
| RegisterEndpointFactory(functionsHostBuilder, serviceBusConfiguration); | ||
| } | ||
|
|
@@ -60,7 +61,7 @@ public static void UseNServiceBus( | |
| Action<ServiceBusTriggeredEndpointConfiguration> configurationFactory = null) | ||
| { | ||
| var config = functionsHostBuilder.GetContext().Configuration; | ||
| var serviceBusConfiguration = new ServiceBusTriggeredEndpointConfiguration(endpointName, config, connectionString); | ||
| var serviceBusConfiguration = new ServiceBusTriggeredEndpointConfiguration(endpointName, config, GetTransportTransactionMode(), connectionString); | ||
| configurationFactory?.Invoke(serviceBusConfiguration); | ||
| RegisterEndpointFactory(functionsHostBuilder, serviceBusConfiguration); | ||
| } | ||
|
|
@@ -109,5 +110,19 @@ internal static IStartableEndpointWithExternallyManagedContainer Configure( | |
| endpointConfiguration, | ||
| serviceCollection); | ||
| } | ||
|
|
||
| static TransportTransactionMode GetTransportTransactionMode() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn't this hugely negatively affect the function startup time? Wouldn't it be better to rethink our current approach of hosting endpoints within functions to avoid having to scan assemblies? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree that the design needs a rethink but we are already doing the reflection for most user already since we pick the endpoint name from the attribute here https://github.com/Particular/NServiceBus.AzureFunctions.InProcess.ServiceBus/blob/master/src/NServiceBus.AzureFunctions.InProcess.ServiceBus/FunctionsHostBuilderExtensions.cs#L24 Perhaps we can make this more efficient somehow? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So by looking at the code you posted, there is an assumption this attribute is placed into the calling assembly. So, why don't we try to get that attribute for now in all those extensions once and then piggyback the information from there into the configuration? Is there a legacy that would prevent us from doing that? |
||
| { | ||
| foreach (var assembly in AppDomain.CurrentDomain.GetAssemblies()) | ||
| { | ||
| var attribute = assembly.GetCustomAttribute<NServiceBusTriggerFunctionAttribute>(); | ||
|
|
||
| if (attribute != null) | ||
| { | ||
| return attribute.SendsAtomicWithReceive ? TransportTransactionMode.SendsAtomicWithReceive : TransportTransactionMode.ReceiveOnly; | ||
| } | ||
| } | ||
| return TransportTransactionMode.SendsAtomicWithReceive; | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
| using AzureFunctions.InProcess.ServiceBus; | ||
| using Configuration.AdvancedExtensibility; | ||
| using Logging; | ||
| using Microsoft.Azure.WebJobs.ServiceBus; | ||
| using Microsoft.Extensions.Configuration; | ||
| using Serialization; | ||
|
|
||
|
|
@@ -40,7 +41,7 @@ static ServiceBusTriggeredEndpointConfiguration() | |
| /// <summary> | ||
| /// Creates a serverless NServiceBus endpoint. | ||
| /// </summary> | ||
| internal ServiceBusTriggeredEndpointConfiguration(string endpointName, IConfiguration configuration, string connectionString = default) | ||
| internal ServiceBusTriggeredEndpointConfiguration(string endpointName, IConfiguration configuration, TransportTransactionMode transportTransactionMode, string connectionString = default) | ||
| { | ||
| var endpointConfiguration = new EndpointConfiguration(endpointName); | ||
|
|
||
|
|
@@ -74,9 +75,23 @@ internal ServiceBusTriggeredEndpointConfiguration(string endpointName, IConfigur | |
| } | ||
| } | ||
|
|
||
| if (transportTransactionMode == TransportTransactionMode.SendsAtomicWithReceive) | ||
| { | ||
| var serviceBusOptions = configuration.GetSection("AzureFunctionsJobHost:extensions:ServiceBus") | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a code first API that would allow not to go over the specific configuration section that could backfire on us here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I couldn't find anything when I did the research and GetSection only accepts a string https://docs.microsoft.com/en-us/dotnet/api/system.configuration.configurationmanager.getsection?view=dotnet-plat-ext-6.0 |
||
| .Get<ServiceBusOptions>(); | ||
|
|
||
| if (serviceBusOptions == null || serviceBusOptions.EnableCrossEntityTransactions) | ||
| { | ||
| throw new Exception("SendsAtomicWithReceive mode requires EnableCrossEntityTransactions needs to be enabled on the ServiceBusOptions."); | ||
| } | ||
| } | ||
|
|
||
| Transport = new AzureServiceBusTransport(connectionString); | ||
|
|
||
| serverlessTransport = new ServerlessTransport(Transport); | ||
| serverlessTransport = new ServerlessTransport(Transport) | ||
| { | ||
| TransportTransactionMode = transportTransactionMode | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't strictly needed since our transport isn't receiving messages but I figured that this would be more future proof if we for some reason add some logic in the outgoing pipeline that makes decisions based on the transaction mode |
||
| }; | ||
| var serverlessRouting = endpointConfiguration.UseTransport(serverlessTransport); | ||
| Routing = new RoutingSettings<AzureServiceBusTransport>(serverlessRouting.GetSettings()); | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.