From 58fc47a11e644ae6b0d0c5f66468d98d7f91e978 Mon Sep 17 00:00:00 2001 From: Matt Ellis Date: Fri, 17 Apr 2020 16:30:30 -0700 Subject: [PATCH] [EventHubs] Enable the FxCop Analysizers As a follow up to #11414, I was trying to understand why did not see errors from the "FxCop Analyiszers", which would have pointed out that issue to us. The reason was simple - we had disabled the analysis for these projects. I've re-enabled them, either fixing up issues or adding suppressions. We ended up catching some reasonable errors (a few cases where we called the ArgumentException constructor but messed up the order of the message and the parameter name) as well as a case where we were passing two parameters to an internal method but then disregarding them. In practice, the code we had was still "correct", because it happened to only be called from a single place today and the only caller was passing the same values that the implementation was actually using. Overall, I think taking this would be worthwhile. It brings this library back in line with the the others. I could imagine some library wide supressions of rules here, specifically the one about not calling virtual methods from constructors, since in practice we are unlikely to hit the problems the rule intends to guard for, but for now I've just made the diff look "as bad as possible" by doing supressions around the offending lines with justifications. --- .../src/Azure.Messaging.EventHubs.Processor.csproj | 1 - .../src/EventProcessorClient.cs | 6 ++++++ .../src/Storage/BlobsCheckpointStore.cs | 8 ++++---- .../src/Amqp/AmqpClient.cs | 3 ++- .../src/Amqp/AmqpConnectionScope.cs | 14 +++++++++----- .../src/Amqp/AmqpConsumer.cs | 7 ++++--- .../src/Amqp/AmqpProducer.cs | 7 ++++--- .../src/Azure.Messaging.EventHubs.csproj | 1 - .../src/Consumer/EventHubConsumerClient.cs | 2 +- .../src/Consumer/EventPosition.cs | 3 ++- .../src/Core/DeveloperCodeException.cs | 2 ++ .../src/EventHubConnection.cs | 9 ++++++++- .../src/EventHubsRetryOptions.cs | 2 ++ .../src/Primitives/EventProcessor{TPartition}.cs | 14 +++++++++++--- .../src/Primitives/PartitionReceiver.cs | 13 ++++++++++++- .../src/Producer/EventHubProducerClient.cs | 2 +- 16 files changed, 68 insertions(+), 26 deletions(-) diff --git a/sdk/eventhub/Azure.Messaging.EventHubs.Processor/src/Azure.Messaging.EventHubs.Processor.csproj b/sdk/eventhub/Azure.Messaging.EventHubs.Processor/src/Azure.Messaging.EventHubs.Processor.csproj index 8ae4628a6e01..039ee6f55183 100755 --- a/sdk/eventhub/Azure.Messaging.EventHubs.Processor/src/Azure.Messaging.EventHubs.Processor.csproj +++ b/sdk/eventhub/Azure.Messaging.EventHubs.Processor/src/Azure.Messaging.EventHubs.Processor.csproj @@ -4,7 +4,6 @@ 5.1.0-preview.2 Azure;Event Hubs;EventHubs;.NET;Event Processor;EventProcessor;$(PackageCommonTags) $(RequiredTargetFrameworks) - false diff --git a/sdk/eventhub/Azure.Messaging.EventHubs.Processor/src/EventProcessorClient.cs b/sdk/eventhub/Azure.Messaging.EventHubs.Processor/src/EventProcessorClient.cs index ce9286210f37..4b594524cca3 100755 --- a/sdk/eventhub/Azure.Messaging.EventHubs.Processor/src/EventProcessorClient.cs +++ b/sdk/eventhub/Azure.Messaging.EventHubs.Processor/src/EventProcessorClient.cs @@ -31,7 +31,9 @@ namespace Azure.Messaging.EventHubs     ///   allowing the processor to be resilient in the face of errors.     ///      /// +#pragma warning disable CA1001 // Types that own disposable fields should be disposable public class EventProcessorClient : EventProcessor +#pragma warning restore CA1001 // Types that own disposable fields should be disposable { /// The number of events to request as the maximum size for batches read from a partition. private const int ReadBatchSize = 15; @@ -66,6 +68,7 @@ public class EventProcessorClient : EventProcessor /// [SuppressMessage("Usage", "AZC0002:Ensure all service methods take an optional CancellationToken parameter.", Justification = "Guidance does not apply; this is an event.")] [SuppressMessage("Usage", "AZC0003:DO make service methods virtual.", Justification = "This member follows the standard .NET event pattern; override via the associated On<> method.")] + [SuppressMessage("Design", "CA1065:Do not raise exceptions in unexpected locations", Justification = "Guidelines do allow throwing NotSupportedException or ArgumentException here")] public event Func PartitionInitializingAsync { add @@ -99,6 +102,7 @@ public event Func PartitionInitializingAsy /// [SuppressMessage("Usage", "AZC0002:Ensure all service methods take an optional CancellationToken parameter.", Justification = "Guidance does not apply; this is an event.")] [SuppressMessage("Usage", "AZC0003:DO make service methods virtual.", Justification = "This member follows the standard .NET event pattern; override via the associated On<> method.")] + [SuppressMessage("Design", "CA1065:Do not raise exceptions in unexpected locations", Justification = "Guidelines do allow throwing NotSupportedException or ArgumentException here")] public event Func PartitionClosingAsync { add @@ -133,6 +137,7 @@ public event Func PartitionClosingAsync /// [SuppressMessage("Usage", "AZC0002:Ensure all service methods take an optional CancellationToken parameter.", Justification = "Guidance does not apply; this is an event.")] [SuppressMessage("Usage", "AZC0003:DO make service methods virtual.", Justification = "This member follows the standard .NET event pattern; override via the associated On<> method.")] + [SuppressMessage("Design", "CA1065:Do not raise exceptions in unexpected locations", Justification = "Guidelines do allow throwing NotSupportedException or ArgumentException here")] public event Func ProcessEventAsync { add @@ -167,6 +172,7 @@ public event Func ProcessEventAsync /// [SuppressMessage("Usage", "AZC0002:Ensure all service methods take an optional CancellationToken parameter.", Justification = "Guidance does not apply; this is an event.")] [SuppressMessage("Usage", "AZC0003:DO make service methods virtual.", Justification = "This member follows the standard .NET event pattern; override via the associated On<> method.")] + [SuppressMessage("Design", "CA1065:Do not raise exceptions in unexpected locations", Justification = "Guidelines do allow throwing NotSupportedException or ArgumentException here")] public event Func ProcessErrorAsync { add diff --git a/sdk/eventhub/Azure.Messaging.EventHubs.Processor/src/Storage/BlobsCheckpointStore.cs b/sdk/eventhub/Azure.Messaging.EventHubs.Processor/src/Storage/BlobsCheckpointStore.cs index 569aece606b0..256f3fd827c7 100755 --- a/sdk/eventhub/Azure.Messaging.EventHubs.Processor/src/Storage/BlobsCheckpointStore.cs +++ b/sdk/eventhub/Azure.Messaging.EventHubs.Processor/src/Storage/BlobsCheckpointStore.cs @@ -99,7 +99,7 @@ public override async Task> ListOw try { - var prefix = string.Format(OwnershipPrefix, fullyQualifiedNamespace.ToLowerInvariant(), eventHubName.ToLowerInvariant(), consumerGroup.ToLowerInvariant()); + var prefix = string.Format(CultureInfo.InvariantCulture, OwnershipPrefix, fullyQualifiedNamespace.ToLowerInvariant(), eventHubName.ToLowerInvariant(), consumerGroup.ToLowerInvariant()); Func>> listOwnershipAsync = async listOwnershipToken => { @@ -168,7 +168,7 @@ public override async Task> ClaimO var blobRequestConditions = new BlobRequestConditions(); - var blobName = string.Format(OwnershipPrefix + ownership.PartitionId, ownership.FullyQualifiedNamespace.ToLowerInvariant(), ownership.EventHubName.ToLowerInvariant(), ownership.ConsumerGroup.ToLowerInvariant()); + var blobName = string.Format(CultureInfo.InvariantCulture, OwnershipPrefix + ownership.PartitionId, ownership.FullyQualifiedNamespace.ToLowerInvariant(), ownership.EventHubName.ToLowerInvariant(), ownership.ConsumerGroup.ToLowerInvariant()); var blobClient = ContainerClient.GetBlobClient(blobName); try @@ -277,7 +277,7 @@ public override async Task> ListCheckpoint cancellationToken.ThrowIfCancellationRequested(); Logger.ListCheckpointsStart(fullyQualifiedNamespace, eventHubName, consumerGroup); - var prefix = string.Format(CheckpointPrefix, fullyQualifiedNamespace.ToLowerInvariant(), eventHubName.ToLowerInvariant(), consumerGroup.ToLowerInvariant()); + var prefix = string.Format(CultureInfo.InvariantCulture, CheckpointPrefix, fullyQualifiedNamespace.ToLowerInvariant(), eventHubName.ToLowerInvariant(), consumerGroup.ToLowerInvariant()); var checkpointCount = 0; Func>> listCheckpointsAsync = async listCheckpointsToken => @@ -359,7 +359,7 @@ public override async Task UpdateCheckpointAsync(EventProcessorCheckpoint checkp cancellationToken.ThrowIfCancellationRequested(); Logger.UpdateCheckpointStart(checkpoint.PartitionId, checkpoint.FullyQualifiedNamespace, checkpoint.EventHubName, checkpoint.ConsumerGroup); - var blobName = string.Format(CheckpointPrefix + checkpoint.PartitionId, checkpoint.FullyQualifiedNamespace.ToLowerInvariant(), checkpoint.EventHubName.ToLowerInvariant(), checkpoint.ConsumerGroup.ToLowerInvariant()); + var blobName = string.Format(CultureInfo.InvariantCulture, CheckpointPrefix + checkpoint.PartitionId, checkpoint.FullyQualifiedNamespace.ToLowerInvariant(), checkpoint.EventHubName.ToLowerInvariant(), checkpoint.ConsumerGroup.ToLowerInvariant()); var blobClient = ContainerClient.GetBlobClient(blobName); var metadata = new Dictionary() diff --git a/sdk/eventhub/Azure.Messaging.EventHubs/src/Amqp/AmqpClient.cs b/sdk/eventhub/Azure.Messaging.EventHubs/src/Amqp/AmqpClient.cs index 340bf5ef14d7..8b1c4760f279 100755 --- a/sdk/eventhub/Azure.Messaging.EventHubs/src/Amqp/AmqpClient.cs +++ b/sdk/eventhub/Azure.Messaging.EventHubs/src/Amqp/AmqpClient.cs @@ -3,6 +3,7 @@ using System; using System.Diagnostics; +using System.Globalization; using System.Security.Authentication; using System.Threading; using System.Threading.Tasks; @@ -462,7 +463,7 @@ public override async Task CloseAsync(CancellationToken cancellationToken) _closed = true; - var clientId = GetHashCode().ToString(); + var clientId = GetHashCode().ToString(CultureInfo.InvariantCulture); var clientType = GetType(); try diff --git a/sdk/eventhub/Azure.Messaging.EventHubs/src/Amqp/AmqpConnectionScope.cs b/sdk/eventhub/Azure.Messaging.EventHubs/src/Amqp/AmqpConnectionScope.cs index a7714a0aeebf..5f50c2bc056e 100755 --- a/sdk/eventhub/Azure.Messaging.EventHubs/src/Amqp/AmqpConnectionScope.cs +++ b/sdk/eventhub/Azure.Messaging.EventHubs/src/Amqp/AmqpConnectionScope.cs @@ -176,9 +176,12 @@ public AmqpConnectionScope(Uri serviceEndpoint, Transport = transport; Proxy = proxy; TokenProvider = new CbsTokenProvider(new EventHubTokenCredential(credential, serviceEndpoint.ToString()), OperationCancellationSource.Token); - Id = identifier ?? $"{ eventHubName }-{ Guid.NewGuid().ToString("D").Substring(0, 8) }"; + Id = identifier ?? $"{ eventHubName }-{ Guid.NewGuid().ToString("D", CultureInfo.InvariantCulture).Substring(0, 8) }"; +#pragma warning disable CA2214 // Do not call overridable methods in constructors. This internal method is virtual for testing purposes. Task connectionFactory(TimeSpan timeout) => CreateAndOpenConnectionAsync(AmqpVersion, ServiceEndpoint, Transport, Proxy, Id, timeout); +#pragma warning restore CA2214 // Do not call overridable methods in constructors + ActiveConnection = new FaultTolerantAmqpObject(connectionFactory, CloseConnection); } @@ -253,7 +256,7 @@ public virtual async Task OpenConsumerLinkAsync(string consum cancellationToken.ThrowIfCancellationRequested(); var stopWatch = Stopwatch.StartNew(); - var consumerEndpoint = new Uri(ServiceEndpoint, string.Format(ConsumerPathSuffixMask, EventHubName, consumerGroup, partitionId)); + var consumerEndpoint = new Uri(ServiceEndpoint, string.Format(CultureInfo.InvariantCulture, ConsumerPathSuffixMask, EventHubName, consumerGroup, partitionId)); var connection = await ActiveConnection.GetOrCreateAsync(timeout).ConfigureAwait(false); cancellationToken.ThrowIfCancellationRequested(); @@ -295,7 +298,7 @@ public virtual async Task OpenProducerLinkAsync(string partitio cancellationToken.ThrowIfCancellationRequested(); var stopWatch = Stopwatch.StartNew(); - var path = (string.IsNullOrEmpty(partitionId)) ? EventHubName : string.Format(PartitionProducerPathSuffixMask, EventHubName, partitionId); + var path = (string.IsNullOrEmpty(partitionId)) ? EventHubName : string.Format(CultureInfo.InvariantCulture, PartitionProducerPathSuffixMask, EventHubName, partitionId); var producerEndpoint = new Uri(ServiceEndpoint, path); var connection = await ActiveConnection.GetOrCreateAsync(timeout).ConfigureAwait(false); @@ -371,10 +374,11 @@ protected virtual async Task CreateAndOpenConnectionAsync(Versio stopWatch.Stop(); +#pragma warning disable CA1806 // Do not ignore method results // Create the CBS link that will be used for authorization. The act of creating the link will associate // it with the connection. - new AmqpCbsLink(connection); +#pragma warning restore CA1806 // Do not ignore method results // When the connection is closed, close each of the links associated with it. @@ -951,7 +955,7 @@ private static void ValidateTransport(EventHubsTransportType transport) { if ((transport != EventHubsTransportType.AmqpTcp) && (transport != EventHubsTransportType.AmqpWebSockets)) { - throw new ArgumentException(nameof(transport), string.Format(CultureInfo.CurrentCulture, Resources.UnknownConnectionType, transport)); + throw new ArgumentException(string.Format(CultureInfo.CurrentCulture, Resources.UnknownConnectionType, transport), nameof(transport)); } } } diff --git a/sdk/eventhub/Azure.Messaging.EventHubs/src/Amqp/AmqpConsumer.cs b/sdk/eventhub/Azure.Messaging.EventHubs/src/Amqp/AmqpConsumer.cs index 02de3ba6f8e4..dcaac1c10ff3 100755 --- a/sdk/eventhub/Azure.Messaging.EventHubs/src/Amqp/AmqpConsumer.cs +++ b/sdk/eventhub/Azure.Messaging.EventHubs/src/Amqp/AmqpConsumer.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Diagnostics; +using System.Globalization; using System.Runtime.ExceptionServices; using System.Threading; using System.Threading.Tasks; @@ -341,7 +342,7 @@ public override async Task CloseAsync(CancellationToken cancellationToken) _closed = true; - var clientId = GetHashCode().ToString(); + var clientId = GetHashCode().ToString(CultureInfo.InvariantCulture); var clientType = GetType(); try @@ -401,12 +402,12 @@ private async Task CreateConsumerLinkAsync(string consumerGro link = await ConnectionScope.OpenConsumerLinkAsync( consumerGroup, partitionId, - CurrentEventPosition, + eventStartingPosition, timeout, prefetchCount, ownerLevel, trackLastEnqueuedEventProperties, - CancellationToken.None).ConfigureAwait(false); + cancellationToken).ConfigureAwait(false); } catch (InvalidOperationException ex) { diff --git a/sdk/eventhub/Azure.Messaging.EventHubs/src/Amqp/AmqpProducer.cs b/sdk/eventhub/Azure.Messaging.EventHubs/src/Amqp/AmqpProducer.cs index 4c38dff72e96..f0269b59918a 100755 --- a/sdk/eventhub/Azure.Messaging.EventHubs/src/Amqp/AmqpProducer.cs +++ b/sdk/eventhub/Azure.Messaging.EventHubs/src/Amqp/AmqpProducer.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Diagnostics; +using System.Globalization; using System.Runtime.ExceptionServices; using System.Threading; using System.Threading.Tasks; @@ -279,7 +280,7 @@ public override async Task CloseAsync(CancellationToken cancellationToken) _closed = true; - var clientId = GetHashCode().ToString(); + var clientId = GetHashCode().ToString(CultureInfo.InvariantCulture); var clientType = GetType(); try @@ -338,7 +339,7 @@ protected virtual async Task SendAsync(Func messageFactory, try { using AmqpMessage batchMessage = messageFactory(); - messageHash = batchMessage.GetHashCode().ToString(); + messageHash = batchMessage.GetHashCode().ToString(CultureInfo.InvariantCulture); // Creation of the link happens without explicit knowledge of the cancellation token // used for this operation; validate the token state before attempting link creation and @@ -355,7 +356,7 @@ protected virtual async Task SendAsync(Func messageFactory, if (batchMessage.SerializedMessageSize > MaximumMessageSize) { - throw new EventHubsException(EventHubName, string.Format(Resources.MessageSizeExceeded, messageHash, batchMessage.SerializedMessageSize, MaximumMessageSize), EventHubsException.FailureReason.MessageSizeExceeded); + throw new EventHubsException(EventHubName, string.Format(CultureInfo.CurrentCulture, Resources.MessageSizeExceeded, messageHash, batchMessage.SerializedMessageSize, MaximumMessageSize), EventHubsException.FailureReason.MessageSizeExceeded); } // Attempt to send the message batch. diff --git a/sdk/eventhub/Azure.Messaging.EventHubs/src/Azure.Messaging.EventHubs.csproj b/sdk/eventhub/Azure.Messaging.EventHubs/src/Azure.Messaging.EventHubs.csproj index 8a9af3a9ac0b..0c99c9e18d07 100755 --- a/sdk/eventhub/Azure.Messaging.EventHubs/src/Azure.Messaging.EventHubs.csproj +++ b/sdk/eventhub/Azure.Messaging.EventHubs/src/Azure.Messaging.EventHubs.csproj @@ -4,7 +4,6 @@ 5.1.0-preview.2 Azure;Event Hubs;EventHubs;.NET;AMQP;IoT;$(PackageCommonTags) $(RequiredTargetFrameworks) - false diff --git a/sdk/eventhub/Azure.Messaging.EventHubs/src/Consumer/EventHubConsumerClient.cs b/sdk/eventhub/Azure.Messaging.EventHubs/src/Consumer/EventHubConsumerClient.cs index ca234dfc36b0..4d148998a4ba 100755 --- a/sdk/eventhub/Azure.Messaging.EventHubs/src/Consumer/EventHubConsumerClient.cs +++ b/sdk/eventhub/Azure.Messaging.EventHubs/src/Consumer/EventHubConsumerClient.cs @@ -601,7 +601,7 @@ public virtual async Task CloseAsync(CancellationToken cancellationToken = defau IsClosed = true; - var clientHash = GetHashCode().ToString(); + var clientHash = GetHashCode().ToString(CultureInfo.InvariantCulture); EventHubsEventSource.Log.ClientCloseStart(typeof(EventHubConsumerClient), EventHubName, clientHash); // Attempt to close the active transport consumers. In the event that an exception is encountered, diff --git a/sdk/eventhub/Azure.Messaging.EventHubs/src/Consumer/EventPosition.cs b/sdk/eventhub/Azure.Messaging.EventHubs/src/Consumer/EventPosition.cs index 10ed6095407d..3991e0a94ccd 100755 --- a/sdk/eventhub/Azure.Messaging.EventHubs/src/Consumer/EventPosition.cs +++ b/sdk/eventhub/Azure.Messaging.EventHubs/src/Consumer/EventPosition.cs @@ -3,6 +3,7 @@ using System; using System.ComponentModel; +using System.Globalization; using Azure.Core; namespace Azure.Messaging.EventHubs.Consumer @@ -85,7 +86,7 @@ public struct EventPosition : IEquatable /// The position of the specified event. /// public static EventPosition FromOffset(long offset, - bool isInclusive = true) => FromOffset(offset.ToString(), isInclusive); + bool isInclusive = true) => FromOffset(offset.ToString(CultureInfo.InvariantCulture), isInclusive); /// /// Corresponds to the event in the partition having a specified sequence number associated with it. diff --git a/sdk/eventhub/Azure.Messaging.EventHubs/src/Core/DeveloperCodeException.cs b/sdk/eventhub/Azure.Messaging.EventHubs/src/Core/DeveloperCodeException.cs index 5a74e0bd7d8a..57df3ac6e6cc 100755 --- a/sdk/eventhub/Azure.Messaging.EventHubs/src/Core/DeveloperCodeException.cs +++ b/sdk/eventhub/Azure.Messaging.EventHubs/src/Core/DeveloperCodeException.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using System; +using System.Diagnostics.CodeAnalysis; namespace Azure.Messaging.EventHubs { @@ -13,6 +14,7 @@ namespace Azure.Messaging.EventHubs /// /// /// + [SuppressMessage("Design", "CA1064:Exceptions should be public", Justification = "This exception is not visible to user code")] internal class DeveloperCodeException : Exception { /// diff --git a/sdk/eventhub/Azure.Messaging.EventHubs/src/EventHubConnection.cs b/sdk/eventhub/Azure.Messaging.EventHubs/src/EventHubConnection.cs index 568bbcca7c5f..74d673907c5b 100755 --- a/sdk/eventhub/Azure.Messaging.EventHubs/src/EventHubConnection.cs +++ b/sdk/eventhub/Azure.Messaging.EventHubs/src/EventHubConnection.cs @@ -172,7 +172,10 @@ public EventHubConnection(string connectionString, FullyQualifiedNamespace = fullyQualifiedNamespace; EventHubName = eventHubName; Options = connectionOptions; + +#pragma warning disable CA2214 // Do not call overridable methods in constructors. This internal method is virtual for testing purposes. InnerClient = CreateTransportClient(fullyQualifiedNamespace, eventHubName, tokenCredentials, connectionOptions); +#pragma warning restore CA2214 // Do not call overridable methods in constructors. } /// @@ -212,7 +215,9 @@ public EventHubConnection(string fullyQualifiedNamespace, EventHubName = eventHubName; Options = connectionOptions; +#pragma warning disable CA2214 // Do not call overridable methods in constructors. This internal method is virtual for testing purposes. InnerClient = CreateTransportClient(fullyQualifiedNamespace, eventHubName, tokenCredential, connectionOptions); +#pragma warning restore CA2214 // Do not call overridable methods in constructors. } /// @@ -427,7 +432,9 @@ internal virtual TransportClient CreateTransportClient(string fullyQualifiedName return new AmqpClient(fullyQualifiedNamespace, eventHubName, credential, options); default: +#pragma warning disable CA2208 // Instantiate argument exceptions correctly. "TransportType" is a reasonable name. It's the property on the options argument which is invalid. throw new ArgumentException(string.Format(CultureInfo.CurrentCulture, Resources.InvalidTransportType, options.TransportType.ToString()), nameof(options.TransportType)); +#pragma warning restore CA2208 // Instantiate argument exceptions correctly } } @@ -455,7 +462,7 @@ private static string BuildAudienceResource(EventHubsTransportType transportType UserName = string.Empty, }; - if (builder.Path.EndsWith("/")) + if (builder.Path.EndsWith("/", StringComparison.Ordinal)) { builder.Path = builder.Path.TrimEnd('/'); } diff --git a/sdk/eventhub/Azure.Messaging.EventHubs/src/EventHubsRetryOptions.cs b/sdk/eventhub/Azure.Messaging.EventHubs/src/EventHubsRetryOptions.cs index b064c31199a5..a071af9bfb00 100755 --- a/sdk/eventhub/Azure.Messaging.EventHubs/src/EventHubsRetryOptions.cs +++ b/sdk/eventhub/Azure.Messaging.EventHubs/src/EventHubsRetryOptions.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using System; +using System.Diagnostics.CodeAnalysis; using Azure.Core; namespace Azure.Messaging.EventHubs @@ -83,6 +84,7 @@ public TimeSpan MaximumDelay /// attempt or a retry. /// /// + [SuppressMessage("Usage", "CA2208:Instantiate argument exceptions correctly", Justification = "We believe using the property name instead of 'value' is more intuitive")] public TimeSpan TryTimeout { get => _tryTimeout; diff --git a/sdk/eventhub/Azure.Messaging.EventHubs/src/Primitives/EventProcessor{TPartition}.cs b/sdk/eventhub/Azure.Messaging.EventHubs/src/Primitives/EventProcessor{TPartition}.cs index 4d4b92b99c41..febcfa6b4a84 100755 --- a/sdk/eventhub/Azure.Messaging.EventHubs/src/Primitives/EventProcessor{TPartition}.cs +++ b/sdk/eventhub/Azure.Messaging.EventHubs/src/Primitives/EventProcessor{TPartition}.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.ComponentModel; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Linq; using System.Runtime.ExceptionServices; @@ -29,6 +30,7 @@ namespace Azure.Messaging.EventHubs.Primitives /// /// The context of the partition for which an operation is being performed. /// + [SuppressMessage("Microsoft.Design", "CA1001:TypesThatOwnDisposableFieldsShouldBeDisposable")] public abstract class EventProcessor where TPartition : EventProcessorPartition, new() { /// The maximum number of failed consumers to allow when processing a partition; failed consumers are those which have been unable to receive and process events. @@ -234,7 +236,10 @@ internal EventProcessor(int eventBatchMaximumCount, RetryPolicy = options.RetryOptions.ToRetryPolicy(); Options = options; EventBatchMaximumCount = eventBatchMaximumCount; + +#pragma warning disable CA2214 // Do not call overridable methods in constructors. The virtual methods are internal and used for testing. LoadBalancer = loadBalancer ?? CreatePartitionLoadBalancer(CreateStorageManager(this), Identifier, ConsumerGroup, FullyQualifiedNamespace, EventHubName, options.PartitionOwnershipExpirationInterval); +#pragma warning restore CA2214 // Do not call overridable methods in constructors. } /// @@ -304,7 +309,10 @@ protected EventProcessor(int eventBatchMaximumCount, RetryPolicy = options.RetryOptions.ToRetryPolicy(); Options = options; EventBatchMaximumCount = eventBatchMaximumCount; + +#pragma warning disable CA2214 // Do not call overridable methods in constructors. The virtual methods are internal and used for testing. LoadBalancer = CreatePartitionLoadBalancer(CreateStorageManager(this), Identifier, ConsumerGroup, FullyQualifiedNamespace, EventHubName, options.PartitionOwnershipExpirationInterval); +#pragma warning restore CA2214 // Do not call overridable methods in constructors } /// @@ -491,7 +499,7 @@ internal virtual async Task ProcessEventBatchAsync(TPartition partition, { var attributes = new Dictionary(1) { - { DiagnosticProperty.EnqueuedTimeAttribute, eventData.EnqueuedTime.ToUnixTimeMilliseconds().ToString() } + { DiagnosticProperty.EnqueuedTimeAttribute, eventData.EnqueuedTime.ToUnixTimeMilliseconds().ToString(CultureInfo.InvariantCulture) } }; diagnosticScope.AddLink(diagnosticId, attributes); @@ -615,7 +623,7 @@ async Task performProcessing() // If the batch was successfully processed, capture the last event as the current starting position, in the // event that the consumer becomes invalid and needs to be replaced. - lastEvent = eventBatch?.LastOrDefault(); + lastEvent = (eventBatch != null && eventBatch.Count > 0) ? eventBatch[eventBatch.Count - 1] : null; if ((lastEvent != null) && (lastEvent.Offset != long.MinValue)) { @@ -1219,7 +1227,7 @@ private async Task PerformLoadBalancingAsync(Stopwatch cycleDuration, var partition = (partitionId ?? string.Empty) switch { - string id when (id == string.Empty) => null, + string id when (id.Length == 0) => null, string _ when (ActivePartitionProcessors.TryGetValue(partitionId, out var partitionProcessor)) => partitionProcessor.Partition, _ => new TPartition { PartitionId = partitionId } }; diff --git a/sdk/eventhub/Azure.Messaging.EventHubs/src/Primitives/PartitionReceiver.cs b/sdk/eventhub/Azure.Messaging.EventHubs/src/Primitives/PartitionReceiver.cs index 17e98f611a2c..b583ff86f58c 100755 --- a/sdk/eventhub/Azure.Messaging.EventHubs/src/Primitives/PartitionReceiver.cs +++ b/sdk/eventhub/Azure.Messaging.EventHubs/src/Primitives/PartitionReceiver.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.ComponentModel; +using System.Globalization; using System.Runtime.ExceptionServices; using System.Threading; using System.Threading.Tasks; @@ -182,7 +183,11 @@ public PartitionReceiver(string consumerGroup, InitialPosition = eventPosition; DefaultMaximumWaitTime = options.DefaultMaximumReceiveWaitTime; RetryPolicy = options.RetryOptions.ToRetryPolicy(); + +#pragma warning disable CA2214 // Do not call overridable methods in constructors. This internal method is virtual for testing purposes. InnerConsumer = CreateTransportConsumer(consumerGroup, partitionId, eventPosition, RetryPolicy, options); +#pragma warning restore CA2214 // Do not call overridable methods in constructors. + } /// @@ -219,7 +224,10 @@ public PartitionReceiver(string consumerGroup, InitialPosition = eventPosition; DefaultMaximumWaitTime = options.DefaultMaximumReceiveWaitTime; RetryPolicy = options.RetryOptions.ToRetryPolicy(); + +#pragma warning disable CA2214 // Do not call overridable methods in constructors. This internal method is virtual for testing purposes. InnerConsumer = CreateTransportConsumer(consumerGroup, partitionId, eventPosition, RetryPolicy, options); +#pragma warning restore CA2214 // Do not call overridable methods in constructors. } /// @@ -251,7 +259,10 @@ public PartitionReceiver(string consumerGroup, InitialPosition = eventPosition; DefaultMaximumWaitTime = options.DefaultMaximumReceiveWaitTime; RetryPolicy = options.RetryOptions.ToRetryPolicy(); + +#pragma warning disable CA2214 // Do not call overridable methods in constructors. This internal method is virtual for testing purposes. InnerConsumer = CreateTransportConsumer(consumerGroup, partitionId, eventPosition, RetryPolicy, options); +#pragma warning restore CA2214 // Do not call overridable methods in constructors. } /// @@ -348,7 +359,7 @@ public virtual async Task CloseAsync(CancellationToken cancellationToken = defau IsClosed = true; - var clientHash = GetHashCode().ToString(); + var clientHash = GetHashCode().ToString(CultureInfo.InvariantCulture); Logger.ClientCloseStart(typeof(PartitionReceiver), EventHubName, clientHash); // Attempt to close the transport consumer. In the event that an exception is encountered, diff --git a/sdk/eventhub/Azure.Messaging.EventHubs/src/Producer/EventHubProducerClient.cs b/sdk/eventhub/Azure.Messaging.EventHubs/src/Producer/EventHubProducerClient.cs index 5725bb8fcc48..f663f412d016 100644 --- a/sdk/eventhub/Azure.Messaging.EventHubs/src/Producer/EventHubProducerClient.cs +++ b/sdk/eventhub/Azure.Messaging.EventHubs/src/Producer/EventHubProducerClient.cs @@ -583,7 +583,7 @@ public virtual async Task CloseAsync(CancellationToken cancellationToken = defau IsClosed = true; - var identifier = GetHashCode().ToString(); + var identifier = GetHashCode().ToString(CultureInfo.InvariantCulture); EventHubsEventSource.Log.ClientCloseStart(typeof(EventHubProducerClient), EventHubName, identifier); // Attempt to close the pool of producers. In the event that an exception is encountered,