Update arg validation

sdk/core/Azure.Core.Amqp/src/AmqpAddress.cs

@@ -22,6 +22,7 @@ public struct AmqpAddress : IEquatable<AmqpAddress>
         /// <param name="address">The address.</param>
         public AmqpAddress(string address)
         {
+            Argument.AssertNotNull(address, nameof(address));
             _address = address;
         }
 

sdk/core/Azure.Core.Amqp/src/AmqpMessageBody.cs

@@ -30,7 +30,8 @@ public class AmqpMessageBody
         /// <param name="data">The data sections.</param>
         public AmqpMessageBody(IEnumerable<ReadOnlyMemory<byte>> data)
         {
-            _data = data ?? Enumerable.Empty<ReadOnlyMemory<byte>>();
+            Argument.AssertNotNull(data, nameof(data));
+            _data = data;
             BodyType = AmqpMessageBodyType.Data;
         }
 

sdk/core/Azure.Core.Amqp/src/AmqpMessageId.cs

@@ -12,7 +12,7 @@ namespace Azure.Core.Amqp
     /// </summary>
     public struct AmqpMessageId : IEquatable<AmqpMessageId>
     {
-        private readonly string? _messageIdString;
+        private readonly string _messageIdString;
 
         /// <summary>
         /// Initializes a new <see cref="AmqpMessageId"/> using the provided
@@ -22,6 +22,7 @@ public struct AmqpMessageId : IEquatable<AmqpMessageId>
         /// <param name="messageId">The message Id.</param>
         public AmqpMessageId(string messageId)
         {
+            Argument.AssertNotNull(messageId, nameof(messageId));
             _messageIdString = messageId;
         }
 
@@ -30,7 +31,7 @@ public AmqpMessageId(string messageId)
         /// </summary>
         ///
         /// <returns>A <see cref="string"/> from the value of this instance.</returns>
-        public override string ToString() => _messageIdString!;
+        public override string ToString() => _messageIdString;
 
         /// <summary>
         /// Determines whether the provided object is equal to the current object.
@@ -69,7 +70,7 @@ public override int GetHashCode()
         /// <see cref="AmqpMessageId"/>; otherwise, <see langword="false" />.
         /// </returns>
         public bool Equals(AmqpMessageId other) =>
-            other.Equals(_messageIdString!);
+            other.Equals(_messageIdString);
 
         /// <summary>
         /// Determines whether the provided <see cref="string"/> is equal to the current instance.

sdk/core/Azure.Core.Amqp/tests/AmqpAddressTests.cs

@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
+using System;
 using NUnit.Framework;
 
 namespace Azure.Core.Amqp.Tests
@@ -17,5 +18,13 @@ public void CanCreateFromString()
             Assert.True(address.Equals((object)new AmqpAddress("address")));
             Assert.False(address.Equals(new AmqpMessageId("messageId2")));
         }
+
+        [Test]
+        public void CannotCreateFromNullAddress()
+        {
+            Assert.That(
+                () => new AmqpAddress(null),
+                Throws.InstanceOf<ArgumentNullException>());
+        }
     }
 }

sdk/core/Azure.Core.Amqp/tests/AmqpDataMessageBodyTests.cs

@@ -2,7 +2,6 @@
 // Licensed under the MIT License.
 
 using System;
-using System.Collections.Generic;
 using NUnit.Framework;
 
 namespace Azure.Core.Amqp.Tests
@@ -16,11 +15,14 @@ public void CanCreateDataBody()
             Assert.AreEqual(AmqpMessageBodyType.Data, body.BodyType);
             Assert.IsTrue(body.TryGetData(out var data));
             Assert.NotNull(data);
+        }
 
-            body = new AmqpMessageBody(null);
-            Assert.AreEqual(AmqpMessageBodyType.Data, body.BodyType);
-            Assert.IsTrue(body.TryGetData(out data));
-            Assert.NotNull(data);
+        [Test]
+        public void CannotCreateFromNullBody()
+        {
+            Assert.That(
+                () => new AmqpMessageBody(null),
+                Throws.InstanceOf<ArgumentNullException>());
         }
     }
 }

sdk/core/Azure.Core.Amqp/tests/AmqpMessageIdTests.cs

@@ -27,5 +27,13 @@ public void CanCreateFromString()
             Assert.False(messageId.Equals(Guid.NewGuid()));
             Assert.False(messageId.Equals((object)"messageId"));
         }
+
+        [Test]
+        public void CannotCreateFromNullMessageId()
+        {
+            Assert.That(
+                () => new AmqpMessageId(null),
+                Throws.InstanceOf<ArgumentNullException>());
+        }
     }
 }

sdk/servicebus/Azure.Messaging.ServiceBus/src/Primitives/ServiceBusModelFactory.cs

@@ -44,11 +44,21 @@ public static ServiceBusReceivedMessage ServiceBusReceivedMessage(
             DateTimeOffset enqueuedTime = default)
         {
             var amqpMessage = new AmqpAnnotatedMessage(new AmqpMessageBody(new ReadOnlyMemory<byte>[] { body }));
-            amqpMessage.Properties.CorrelationId = new AmqpMessageId(correlationId);
+
+            if (correlationId != default)
+            {
+                amqpMessage.Properties.CorrelationId = new AmqpMessageId(correlationId);
+            }
             amqpMessage.Properties.Subject = subject;
-            amqpMessage.Properties.To = new AmqpAddress(to);
+            if (to != default)
+            {
+                amqpMessage.Properties.To = new AmqpAddress(to);
+            }
             amqpMessage.Properties.ContentType = contentType;
-            amqpMessage.Properties.ReplyTo = new AmqpAddress(replyTo);
+            if (replyTo != default)
+            {
+                amqpMessage.Properties.ReplyTo = new AmqpAddress(replyTo);
+            }
             amqpMessage.MessageAnnotations[AmqpMessageConstants.ScheduledEnqueueTimeUtcName] = scheduledEnqueueTime.UtcDateTime;
 
             if (messageId != default)

sdk/servicebus/Azure.Messaging.ServiceBus/src/Receiver/ServiceBusReceiver.cs

@@ -423,6 +423,8 @@ private async Task<IReadOnlyList<ServiceBusReceivedMessage>> PeekMessagesInterna
             CancellationToken cancellationToken)
         {
             Argument.AssertNotDisposed(IsClosed, nameof(ServiceBusReceiver));
+            Argument.AssertAtLeast(maxMessages, 1, nameof(maxMessages));
+
             cancellationToken.ThrowIfCancellationRequested<TaskCanceledException>();
             Logger.PeekMessageStart(Identifier, sequenceNumber, maxMessages);
             using DiagnosticScope scope = ScopeFactory.CreateScope(
@@ -898,9 +900,13 @@ public virtual async Task<IReadOnlyList<ServiceBusReceivedMessage>> ReceiveDefer
             CancellationToken cancellationToken = default)
         {
             Argument.AssertNotDisposed(IsClosed, nameof(ServiceBusReceiver));
-            Argument.AssertNotNullOrEmpty(sequenceNumbers, nameof(sequenceNumbers));
+            Argument.AssertNotNull(sequenceNumbers, nameof(sequenceNumbers));
             cancellationToken.ThrowIfCancellationRequested<TaskCanceledException>();
             var sequenceNumbersList = sequenceNumbers.ToList();
+            if (sequenceNumbersList.Count == 0)
+            {
+                return Array.Empty<ServiceBusReceivedMessage>();
+            }
 
             Logger.ReceiveDeferredMessageStart(Identifier, sequenceNumbersList);
             using DiagnosticScope scope = ScopeFactory.CreateScope(DiagnosticProperty.ReceiveDeferredActivityName);

sdk/servicebus/Azure.Messaging.ServiceBus/src/Sender/ServiceBusSender.cs

@@ -428,10 +428,15 @@ public virtual async Task<long[]> ScheduleMessagesAsync(
             DateTimeOffset scheduledEnqueueTime,
             CancellationToken cancellationToken = default)
         {
-            Argument.AssertNotNullOrEmpty(messages, nameof(messages));
+            Argument.AssertNotNull(messages, nameof(messages));
             Argument.AssertNotDisposed(IsClosed, nameof(ServiceBusSender));
             cancellationToken.ThrowIfCancellationRequested<TaskCanceledException>();
             var messageList = messages.ToList();
+            if (messageList.Count == 0)
+            {
+                return Array.Empty<long>();
+            }
+
             await ApplyPlugins(messageList).ConfigureAwait(false);
             Logger.ScheduleMessagesStart(
                 Identifier,
@@ -489,8 +494,14 @@ public virtual async Task CancelScheduledMessagesAsync(
         {
             Argument.AssertNotDisposed(IsClosed, nameof(ServiceBusSender));
             cancellationToken.ThrowIfCancellationRequested<TaskCanceledException>();
-            var sequenceNumberList = sequenceNumbers.ToArray();
-            Logger.CancelScheduledMessagesStart(Identifier, sequenceNumberList);
+            long[] sequenceNumbersArray = sequenceNumbers.ToArray();
+
+            if (sequenceNumbersArray.Length == 0)
+            {
+                return;
+            }
+
+            Logger.CancelScheduledMessagesStart(Identifier, sequenceNumbersArray);
             using DiagnosticScope scope = _scopeFactory.CreateScope(
                 DiagnosticProperty.CancelActivityName,
                 DiagnosticProperty.ClientKind);
@@ -499,7 +510,7 @@ public virtual async Task CancelScheduledMessagesAsync(
             scope.Start();
             try
             {
-                await _innerSender.CancelScheduledMessagesAsync(sequenceNumberList, cancellationToken).ConfigureAwait(false);
+                await _innerSender.CancelScheduledMessagesAsync(sequenceNumbersArray, cancellationToken).ConfigureAwait(false);
             }
             catch (Exception ex)
             {

sdk/servicebus/Azure.Messaging.ServiceBus/tests/Receiver/ReceiverLiveTests.cs

@@ -362,6 +362,11 @@ public async Task DeferMessages()
                 Assert.That(
                     async () => await receiver.ReceiveDeferredMessagesAsync(sequenceNumbers),
                     Throws.InstanceOf<ServiceBusException>().And.Property(nameof(ServiceBusException.Reason)).EqualTo(ServiceBusFailureReason.MessageNotFound));
+
+                // verify that an empty list can be passed
+
+                deferredMessages = await receiver.ReceiveDeferredMessagesAsync(Array.Empty<long>());
+                Assert.IsEmpty(deferredMessages);
             }
         }
 

sdk/servicebus/Azure.Messaging.ServiceBus/tests/Receiver/ReceiverTests.cs

@@ -56,6 +56,38 @@ public void EntityPathConstructedCorrectly()
             Assert.AreEqual("queueName/$Transfer/$DeadLetterQueue", receiver.EntityPath);
         }
 
+        [Test]
+        public void PeekValidatesMaxMessageCount()
+        {
+            var account = Encoding.Default.GetString(GetRandomBuffer(12));
+            var fullyQualifiedNamespace = new UriBuilder($"{account}.servicebus.windows.net/").Host;
+            var connString = $"Endpoint=sb://{fullyQualifiedNamespace};SharedAccessKeyName=RootManageSharedAccessKey;SharedAccessKey={Encoding.Default.GetString(GetRandomBuffer(64))}";
+            var client = new ServiceBusClient(connString);
+            var receiver = client.CreateReceiver("queueName");
+            Assert.That(
+                async () => await receiver.PeekMessagesAsync(0),
+                Throws.InstanceOf<ArgumentOutOfRangeException>());
+            Assert.That(
+                async () => await receiver.PeekMessagesAsync(-1),
+                Throws.InstanceOf<ArgumentOutOfRangeException>());
+        }
+
+        [Test]
+        public void ReceiveValidatesMaxMessageCount()
+        {
+            var account = Encoding.Default.GetString(GetRandomBuffer(12));
+            var fullyQualifiedNamespace = new UriBuilder($"{account}.servicebus.windows.net/").Host;
+            var connString = $"Endpoint=sb://{fullyQualifiedNamespace};SharedAccessKeyName=RootManageSharedAccessKey;SharedAccessKey={Encoding.Default.GetString(GetRandomBuffer(64))}";
+            var client = new ServiceBusClient(connString);
+            var receiver = client.CreateReceiver("queueName");
+            Assert.That(
+                async () => await receiver.ReceiveMessagesAsync(0),
+                Throws.InstanceOf<ArgumentOutOfRangeException>());
+            Assert.That(
+                async () => await receiver.ReceiveMessagesAsync(-1),
+                Throws.InstanceOf<ArgumentOutOfRangeException>());
+        }
+
         [Test]
         public void ReceiverOptionsValidation()
         {

sdk/servicebus/Azure.Messaging.ServiceBus/tests/Sender/SenderLiveTests.cs

@@ -241,6 +241,9 @@ public async Task ScheduleMultiple()
                     ServiceBusReceivedMessage msg = await receiver.PeekMessageAsync(seq);
                     Assert.IsNull(msg);
                 }
+
+                // can cancel empty array
+                await sender.CancelScheduledMessagesAsync(sequenceNumbers: Array.Empty<long>());
             }
         }
 

sdk/servicebus/Azure.Messaging.ServiceBus/tests/Sender/SenderTests.cs

@@ -94,15 +94,17 @@ public void ScheduleNullMessageListShouldThrow()
         }
 
         [Test]
-        public void ScheduleEmptyListShouldThrow()
+        public async Task ScheduleEmptyListShouldNotThrow()
         {
             var mock = new Mock<ServiceBusSender>()
             {
                 CallBase = true
             };
-            Assert.ThrowsAsync<ArgumentException>(async () => await mock.Object.ScheduleMessagesAsync(
+
+            long[] sequenceNums = await mock.Object.ScheduleMessagesAsync(
                 new List<ServiceBusMessage>(),
-                default));
+                default);
+            Assert.IsEmpty(sequenceNums);
         }
 
         [Test]