From eeb051118baadd0b4186462bdea666fdd49dc59f Mon Sep 17 00:00:00 2001 From: Jo Palac Date: Wed, 13 Sep 2023 15:48:26 +1000 Subject: [PATCH] Comments from review --- ...Tests.cs => ConfigurationAnalyzerTests.cs} | 4 +- ...s => ConfigurationAnalyzerTestsCSharp8.cs} | 2 +- ...alyzerTests.cs => OptionsAnalyzerTests.cs} | 6 +-- ...=> TransportConfigurationAnalyzerTests.cs} | 27 +++++++++++++- .../AzureFunctionsDiagnostics.cs | 37 ++++++++++++------- ...onAnalyzer.cs => ConfigurationAnalyzer.cs} | 15 +++++--- 6 files changed, 60 insertions(+), 31 deletions(-) rename src/NServiceBus.AzureFunctions.Analyzer.Tests/{AzureFunctionsConfigurationAnalyzerTests.cs => ConfigurationAnalyzerTests.cs} (95%) rename src/NServiceBus.AzureFunctions.Analyzer.Tests/{AzureFunctionsConfigurationAnalyzerTestsCSharp8.cs => ConfigurationAnalyzerTestsCSharp8.cs} (90%) rename src/NServiceBus.AzureFunctions.Analyzer.Tests/{AzureFunctionsSendReplyOptionsAnalyzerTests.cs => OptionsAnalyzerTests.cs} (79%) rename src/NServiceBus.AzureFunctions.Analyzer.Tests/{AzureFunctionsTransportAnalyzerTests.cs => TransportConfigurationAnalyzerTests.cs} (62%) rename src/NServiceBus.AzureFunctions.Analyzer/{AzureFunctionsConfigurationAnalyzer.cs => ConfigurationAnalyzer.cs} (92%) diff --git a/src/NServiceBus.AzureFunctions.Analyzer.Tests/AzureFunctionsConfigurationAnalyzerTests.cs b/src/NServiceBus.AzureFunctions.Analyzer.Tests/ConfigurationAnalyzerTests.cs similarity index 95% rename from src/NServiceBus.AzureFunctions.Analyzer.Tests/AzureFunctionsConfigurationAnalyzerTests.cs rename to src/NServiceBus.AzureFunctions.Analyzer.Tests/ConfigurationAnalyzerTests.cs index 90e0d65c..ffca45aa 100644 --- a/src/NServiceBus.AzureFunctions.Analyzer.Tests/AzureFunctionsConfigurationAnalyzerTests.cs +++ b/src/NServiceBus.AzureFunctions.Analyzer.Tests/ConfigurationAnalyzerTests.cs @@ -1,13 +1,11 @@ namespace NServiceBus.AzureFunctions.Analyzer.Tests { - using System.Threading; - using System; using System.Threading.Tasks; using NUnit.Framework; using static AzureFunctionsDiagnostics; [TestFixture] - public class AzureFunctionsConfigurationAnalyzerTests : AnalyzerTestFixture + public class ConfigurationAnalyzerTests : AnalyzerTestFixture { [TestCase("DefineCriticalErrorAction((errorContext, cancellationToken) => Task.CompletedTask)", DefineCriticalErrorActionNotAllowedId)] [TestCase("LimitMessageProcessingConcurrencyTo(5)", LimitMessageProcessingToNotAllowedId)] diff --git a/src/NServiceBus.AzureFunctions.Analyzer.Tests/AzureFunctionsConfigurationAnalyzerTestsCSharp8.cs b/src/NServiceBus.AzureFunctions.Analyzer.Tests/ConfigurationAnalyzerTestsCSharp8.cs similarity index 90% rename from src/NServiceBus.AzureFunctions.Analyzer.Tests/AzureFunctionsConfigurationAnalyzerTestsCSharp8.cs rename to src/NServiceBus.AzureFunctions.Analyzer.Tests/ConfigurationAnalyzerTestsCSharp8.cs index 80dca22f..cce95f33 100644 --- a/src/NServiceBus.AzureFunctions.Analyzer.Tests/AzureFunctionsConfigurationAnalyzerTestsCSharp8.cs +++ b/src/NServiceBus.AzureFunctions.Analyzer.Tests/ConfigurationAnalyzerTestsCSharp8.cs @@ -6,7 +6,7 @@ using static AzureFunctionsDiagnostics; [TestFixture] - public class AzureFunctionsConfigurationAnalyzerTestsCSharp8 : AnalyzerTestFixture + public class ConfigurationAnalyzerTestsCSharp8 : AnalyzerTestFixture { // HINT: In C# 7 this call is ambiguous with the LearningTransport version as the compiler cannot differentiate method calls via generic type constraints [TestCase("UseTransport()", UseTransportNotAllowedId)] diff --git a/src/NServiceBus.AzureFunctions.Analyzer.Tests/AzureFunctionsSendReplyOptionsAnalyzerTests.cs b/src/NServiceBus.AzureFunctions.Analyzer.Tests/OptionsAnalyzerTests.cs similarity index 79% rename from src/NServiceBus.AzureFunctions.Analyzer.Tests/AzureFunctionsSendReplyOptionsAnalyzerTests.cs rename to src/NServiceBus.AzureFunctions.Analyzer.Tests/OptionsAnalyzerTests.cs index e2cd6683..8ffc2493 100644 --- a/src/NServiceBus.AzureFunctions.Analyzer.Tests/AzureFunctionsSendReplyOptionsAnalyzerTests.cs +++ b/src/NServiceBus.AzureFunctions.Analyzer.Tests/OptionsAnalyzerTests.cs @@ -5,12 +5,10 @@ namespace NServiceBus.AzureFunctions.Analyzer.Tests using static AzureFunctionsDiagnostics; [TestFixture] - public class AzureFunctionsSendReplyOptionsAnalyzerTests : AnalyzerTestFixture + public class OptionsAnalyzerTests : AnalyzerTestFixture { - [TestCase("SendOptions", "RouteReplyToAnyInstance", RouteReplyToAnyInstanceNotAllowedId)] [TestCase("SendOptions", "RouteReplyToThisInstance", RouteReplyToThisInstanceNotAllowedId)] [TestCase("SendOptions", "RouteToThisInstance", RouteToThisInstanceNotAllowedId)] - [TestCase("ReplyOptions", "RouteReplyToAnyInstance", RouteReplyToAnyInstanceNotAllowedId)] [TestCase("ReplyOptions", "RouteReplyToThisInstance", RouteReplyToThisInstanceNotAllowedId)] public Task DiagnosticIsReportedForOptions(string optionsType, string method, string diagnosticId) { @@ -27,8 +25,6 @@ void Bar({optionsType} options) return Assert(diagnosticId, source); } - - [TestCase("SomeOtherClass", "RouteReplyToAnyInstance", RouteReplyToAnyInstanceNotAllowedId)] [TestCase("SomeOtherClass", "RouteReplyToThisInstance", RouteReplyToThisInstanceNotAllowedId)] [TestCase("SomeOtherClass", "RouteToThisInstance", RouteToThisInstanceNotAllowedId)] public Task DiagnosticIsNotReportedForOtherOptions(string optionsType, string method, string diagnosticId) diff --git a/src/NServiceBus.AzureFunctions.Analyzer.Tests/AzureFunctionsTransportAnalyzerTests.cs b/src/NServiceBus.AzureFunctions.Analyzer.Tests/TransportConfigurationAnalyzerTests.cs similarity index 62% rename from src/NServiceBus.AzureFunctions.Analyzer.Tests/AzureFunctionsTransportAnalyzerTests.cs rename to src/NServiceBus.AzureFunctions.Analyzer.Tests/TransportConfigurationAnalyzerTests.cs index 51dfcdc2..264709b2 100644 --- a/src/NServiceBus.AzureFunctions.Analyzer.Tests/AzureFunctionsTransportAnalyzerTests.cs +++ b/src/NServiceBus.AzureFunctions.Analyzer.Tests/TransportConfigurationAnalyzerTests.cs @@ -5,14 +5,16 @@ namespace NServiceBus.AzureFunctions.Analyzer.Tests using static AzureFunctionsDiagnostics; [TestFixture] - public class AzureFunctionsTransportAnalyzerTests : AnalyzerTestFixture + public class TransportConfigurationAnalyzerTests : AnalyzerTestFixture { + [TestCase("TransportTransactionMode", "TransportTransactionMode.None", TransportTransactionModeNotAllowedId)] + [TestCase("EnablePartitioning", "true", EnablePartitioningNotAllowedId)] [TestCase("EntityMaximumSize", "5", EntityMaximumSizeNotAllowedId)] [TestCase("MaxAutoLockRenewalDuration", "new System.TimeSpan(0, 0, 5, 0)", MaxAutoLockRenewalDurationNotAllowedId)] [TestCase("PrefetchCount", "5", PrefetchCountNotAllowedId)] [TestCase("PrefetchMultiplier", "5", PrefetchMultiplierNotAllowedId)] [TestCase("TimeToWaitBeforeTriggeringCircuitBreaker", "new System.TimeSpan(0, 0, 5, 0)", TimeToWaitBeforeTriggeringCircuitBreakerNotAllowedId)] - public Task DiagnosticIsReportedTransportConfiguration(string configName, string configValue, string diagnosticId) + public Task DiagnosticIsReportedTransportConfigurationDirect(string configName, string configValue, string diagnosticId) { var source = $@"using NServiceBus; @@ -27,7 +29,26 @@ void Direct(ServiceBusTriggeredEndpointConfiguration endpointConfig) var transportConfig = endpointConfig.Transport; [|transportConfig.{configName}|] = {configValue}; }} +}}"; + + return Assert(diagnosticId, source); + } + [TestCase("Transactions", "TransportTransactionMode.None", TransportTransactionModeNotAllowedId)] + [TestCase("EnablePartitioning", "", EnablePartitioningNotAllowedId)] + [TestCase("EntityMaximumSize", "5", EntityMaximumSizeNotAllowedId)] + [TestCase("MaxAutoLockRenewalDuration", "new System.TimeSpan(0, 0, 5, 0)", MaxAutoLockRenewalDurationNotAllowedId)] + [TestCase("PrefetchCount", "5", PrefetchCountNotAllowedId)] + [TestCase("PrefetchMultiplier", "5", PrefetchMultiplierNotAllowedId)] + [TestCase("TimeToWaitBeforeTriggeringCircuitBreaker", "new System.TimeSpan(0, 0, 5, 0)", TimeToWaitBeforeTriggeringCircuitBreakerNotAllowedId)] + public Task DiagnosticIsReportedTransportConfigurationExtension(string configName, string configValue, string diagnosticId) + { + var source = + $@"using NServiceBus; +using System; +using System.Threading.Tasks; +class Foo +{{ void Extension(TransportExtensions transportExtension) {{ [|transportExtension.{configName}({configValue})|]; @@ -37,6 +58,7 @@ void Extension(TransportExtensions transportExtension) return Assert(diagnosticId, source); } + [TestCase("EnablePartitioning", "true", EnablePartitioningNotAllowedId)] [TestCase("EntityMaximumSize", "5", EntityMaximumSizeNotAllowedId)] [TestCase("MaxAutoLockRenewalDuration", "new System.TimeSpan(0, 0, 5, 0)", MaxAutoLockRenewalDurationNotAllowedId)] [TestCase("PrefetchCount", "5", PrefetchCountNotAllowedId)] @@ -52,6 +74,7 @@ public Task DiagnosticIsNotReportedForNonTransportConfiguration(string configNam class SomeOtherClass {{ internal int EntityMaximumSize {{ get; set; }} + internal bool EnablePartitioning {{ get; set; }} internal TimeSpan MaxAutoLockRenewalDuration {{ get; set; }} internal int PrefetchCount {{ get; set; }} internal int PrefetchMultiplier {{ get; set; }} diff --git a/src/NServiceBus.AzureFunctions.Analyzer/AzureFunctionsDiagnostics.cs b/src/NServiceBus.AzureFunctions.Analyzer/AzureFunctionsDiagnostics.cs index 27a011a9..9d944d6f 100644 --- a/src/NServiceBus.AzureFunctions.Analyzer/AzureFunctionsDiagnostics.cs +++ b/src/NServiceBus.AzureFunctions.Analyzer/AzureFunctionsDiagnostics.cs @@ -13,21 +13,21 @@ public static class AzureFunctionsDiagnostics public const string OverrideLocalAddressNotAllowedId = "NSBFUNC009"; public const string RouteReplyToThisInstanceNotAllowedId = "NSBFUNC010"; public const string RouteToThisInstanceNotAllowedId = "NSBFUNC011"; - public const string RouteReplyToAnyInstanceNotAllowedId = "NSBFUNC012"; - + public const string TransportTransactionModeNotAllowedId = "NSBFUNC012"; public const string MaxAutoLockRenewalDurationNotAllowedId = "NSBFUNC013"; public const string PrefetchCountNotAllowedId = "NSBFUNC014"; public const string PrefetchMultiplierNotAllowedId = "NSBFUNC015"; public const string TimeToWaitBeforeTriggeringCircuitBreakerNotAllowedId = "NSBFUNC016"; public const string EntityMaximumSizeNotAllowedId = "NSBFUNC017"; + public const string EnablePartitioningNotAllowedId = "NSBFUNC018"; const string DiagnosticCategory = "NServiceBus.AzureFunctions"; internal static readonly DiagnosticDescriptor PurgeOnStartupNotAllowed = new DiagnosticDescriptor( id: PurgeOnStartupNotAllowedId, title: "PurgeOnStartup is not supported in Azure Functions", - messageFormat: "Azure Functions endpoints are started when the first message arrives. PurgeOnStartup may purge whenever a new instance is started.", + messageFormat: "Azure Functions endpoints do not support PurgeOnStartup.", category: DiagnosticCategory, defaultSeverity: DiagnosticSeverity.Error, isEnabledByDefault: true @@ -36,7 +36,7 @@ public static class AzureFunctionsDiagnostics internal static readonly DiagnosticDescriptor LimitMessageProcessingToNotAllowed = new DiagnosticDescriptor( id: LimitMessageProcessingToNotAllowedId, title: "LimitMessageProcessing is not supported in Azure Functions", - messageFormat: "Azure Functions endpoints do not control the message receiver and cannot limit message processing concurrency.", + messageFormat: "Concurrency-related settings are controlled via the Azure Function host.json configuration file.", category: DiagnosticCategory, defaultSeverity: DiagnosticSeverity.Error, isEnabledByDefault: true @@ -81,7 +81,7 @@ public static class AzureFunctionsDiagnostics internal static readonly DiagnosticDescriptor OverrideLocalAddressNotAllowed = new DiagnosticDescriptor( id: OverrideLocalAddressNotAllowedId, title: "OverrideLocalAddress is not supported in Azure Functions", - messageFormat: "Azure Functions endpoints do not control the message receiver and cannot decide the local address.", + messageFormat: "The NServiceBus endpoint address in Azure Functions is determined by the ServiceBusTrigger attribute.", category: DiagnosticCategory, defaultSeverity: DiagnosticSeverity.Error, isEnabledByDefault: true @@ -105,15 +105,6 @@ public static class AzureFunctionsDiagnostics isEnabledByDefault: true ); - internal static readonly DiagnosticDescriptor RouteReplyToAnyInstanceNotAllowed = new DiagnosticDescriptor( - id: RouteReplyToAnyInstanceNotAllowedId, - title: "RouteReplyToAnyInstance is not supported in Azure Functions", - messageFormat: "Azure Functions endpoints do not control the message receiver and by default route the replies to any instance.", - category: DiagnosticCategory, - defaultSeverity: DiagnosticSeverity.Warning, - isEnabledByDefault: true - ); - internal static readonly DiagnosticDescriptor MaxAutoLockRenewalDurationNotAllowed = new DiagnosticDescriptor( id: MaxAutoLockRenewalDurationNotAllowedId, title: "MaxAutoLockRenewalDuration is not supported in Azure Functions", @@ -158,5 +149,23 @@ public static class AzureFunctionsDiagnostics defaultSeverity: DiagnosticSeverity.Error, isEnabledByDefault: true ); + + internal static readonly DiagnosticDescriptor EnablePartitioningNotAllowed = new DiagnosticDescriptor( + id: EnablePartitioningNotAllowedId, + title: "EnablePartitioning is not supported in Azure Functions", + messageFormat: "Azure Functions endpoints do not support automatic queue creation.", + category: DiagnosticCategory, + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true + ); + + internal static readonly DiagnosticDescriptor TransportTransactionModeNotAllowed = new DiagnosticDescriptor( + id: TransportTransactionModeNotAllowedId, + title: "TransportTransactionMode is not supported in Azure Functions", + messageFormat: "Transport TransactionMode is controlled by the Azure Service Bus trigger and cannot be configured via the NServiceBus transport configuration API when using Azure Functions.", + category: DiagnosticCategory, + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true + ); } } \ No newline at end of file diff --git a/src/NServiceBus.AzureFunctions.Analyzer/AzureFunctionsConfigurationAnalyzer.cs b/src/NServiceBus.AzureFunctions.Analyzer/ConfigurationAnalyzer.cs similarity index 92% rename from src/NServiceBus.AzureFunctions.Analyzer/AzureFunctionsConfigurationAnalyzer.cs rename to src/NServiceBus.AzureFunctions.Analyzer/ConfigurationAnalyzer.cs index c5bf9172..1c8b1ab6 100644 --- a/src/NServiceBus.AzureFunctions.Analyzer/AzureFunctionsConfigurationAnalyzer.cs +++ b/src/NServiceBus.AzureFunctions.Analyzer/ConfigurationAnalyzer.cs @@ -9,7 +9,7 @@ namespace NServiceBus.AzureFunctions.Analyzer using NServiceBus.AzureFunctions.Analyzer.Extensions; [DiagnosticAnalyzer(LanguageNames.CSharp)] - public class AzureFunctionsConfigurationAnalyzer : DiagnosticAnalyzer + public class ConfigurationAnalyzer : DiagnosticAnalyzer { public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create( AzureFunctionsDiagnostics.PurgeOnStartupNotAllowed, @@ -21,12 +21,13 @@ public class AzureFunctionsConfigurationAnalyzer : DiagnosticAnalyzer AzureFunctionsDiagnostics.OverrideLocalAddressNotAllowed, AzureFunctionsDiagnostics.RouteReplyToThisInstanceNotAllowed, AzureFunctionsDiagnostics.RouteToThisInstanceNotAllowed, - AzureFunctionsDiagnostics.RouteReplyToAnyInstanceNotAllowed, AzureFunctionsDiagnostics.MaxAutoLockRenewalDurationNotAllowed, AzureFunctionsDiagnostics.PrefetchCountNotAllowed, AzureFunctionsDiagnostics.PrefetchMultiplierNotAllowed, AzureFunctionsDiagnostics.TimeToWaitBeforeTriggeringCircuitBreakerNotAllowed, - AzureFunctionsDiagnostics.EntityMaximumSizeNotAllowed + AzureFunctionsDiagnostics.EntityMaximumSizeNotAllowed, + AzureFunctionsDiagnostics.EnablePartitioningNotAllowed, + AzureFunctionsDiagnostics.TransportTransactionModeNotAllowed ); static readonly Dictionary NotAllowedEndpointConfigurationMethods @@ -46,7 +47,6 @@ static readonly Dictionary NotAllowedSendAndReplyO { ["RouteReplyToThisInstance"] = AzureFunctionsDiagnostics.RouteReplyToThisInstanceNotAllowed, ["RouteToThisInstance"] = AzureFunctionsDiagnostics.RouteToThisInstanceNotAllowed, - ["RouteReplyToAnyInstance"] = AzureFunctionsDiagnostics.RouteReplyToAnyInstanceNotAllowed }; static readonly Dictionary NotAllowedTransportSettings @@ -56,7 +56,10 @@ static readonly Dictionary NotAllowedTransportSett ["PrefetchCount"] = AzureFunctionsDiagnostics.PrefetchCountNotAllowed, ["PrefetchMultiplier"] = AzureFunctionsDiagnostics.PrefetchMultiplierNotAllowed, ["TimeToWaitBeforeTriggeringCircuitBreaker"] = AzureFunctionsDiagnostics.TimeToWaitBeforeTriggeringCircuitBreakerNotAllowed, - ["EntityMaximumSize"] = AzureFunctionsDiagnostics.EntityMaximumSizeNotAllowed + ["EntityMaximumSize"] = AzureFunctionsDiagnostics.EntityMaximumSizeNotAllowed, + ["EnablePartitioning"] = AzureFunctionsDiagnostics.EnablePartitioningNotAllowed, + ["TransportTransactionMode"] = AzureFunctionsDiagnostics.TransportTransactionModeNotAllowed, + ["Transactions"] = AzureFunctionsDiagnostics.TransportTransactionModeNotAllowed }; public override void Initialize(AnalysisContext context) @@ -106,7 +109,7 @@ static void AnalyzeTransport(SyntaxNodeAnalysisContext context) return; } - if (propertySymbol.ContainingType.ToString() == "NServiceBus.AzureServiceBusTransport") + if (propertySymbol.ContainingType.ToString() == "NServiceBus.AzureServiceBusTransport" || propertySymbol.ContainingType.ToString() == "NServiceBus.Transport.TransportDefinition") { context.ReportDiagnostic(diagnosticDescriptor, memberAccess);