[Service Bus] maxMessageCount validation updates (Azure#12453)

Author: ramya-rao-a

sdk/servicebus/service-bus/src/core/managementClient.ts

@@ -35,6 +35,7 @@ import { LinkEntity, RequestResponseLinkOptions } from "./linkEntity";
 import { managementClientLogger, receiverLogger, senderLogger, ServiceBusLogger } from "../log";
 import { toBuffer } from "../util/utils";
 import {
+  InvalidMaxMessageCountError,
   throwErrorIfConnectionClosed,
   throwTypeErrorIfParameterIsEmptyString,
   throwTypeErrorIfParameterMissing,
@@ -384,7 +385,7 @@ export class ManagementClient extends LinkEntity<RequestResponseLink> {
    * @returns Promise<ReceivedSBMessage[]>
    */
   async peek(
-    messageCount?: number,
+    messageCount: number,
     options?: OperationOptionsBase & SendManagementRequestOptions
   ): Promise<ServiceBusReceivedMessage[]> {
     throwErrorIfConnectionClosed(this._context);
@@ -411,7 +412,7 @@ export class ManagementClient extends LinkEntity<RequestResponseLink> {
    */
   async peekMessagesBySession(
     sessionId: string,
-    messageCount?: number,
+    messageCount: number,
     options?: OperationOptionsBase & SendManagementRequestOptions
   ): Promise<ServiceBusReceivedMessage[]> {
     throwErrorIfConnectionClosed(this._context);
@@ -433,7 +434,7 @@ export class ManagementClient extends LinkEntity<RequestResponseLink> {
    */
   async peekBySequenceNumber(
     fromSequenceNumber: Long,
-    maxMessageCount?: number,
+    maxMessageCount: number,
     sessionId?: string,
     options?: OperationOptionsBase & SendManagementRequestOptions
   ): Promise<ServiceBusReceivedMessage[]> {
@@ -445,13 +446,20 @@ export class ManagementClient extends LinkEntity<RequestResponseLink> {
     throwTypeErrorIfParameterNotLong(connId, "fromSequenceNumber", fromSequenceNumber);
 
     // Checks for maxMessageCount
-    if (maxMessageCount !== undefined) {
-      throwTypeErrorIfParameterTypeMismatch(connId, "maxMessageCount", maxMessageCount, "number");
-      if (maxMessageCount <= 0) {
-        return [];
-      }
-    } else {
-      maxMessageCount = 1;
+    throwTypeErrorIfParameterMissing(
+      this._context.connectionId,
+      "maxMessageCount",
+      maxMessageCount
+    );
+    throwTypeErrorIfParameterTypeMismatch(
+      this._context.connectionId,
+      "maxMessageCount",
+      maxMessageCount,
+      "number"
+    );
+
+    if (isNaN(maxMessageCount) || maxMessageCount < 1) {
+      throw new TypeError(InvalidMaxMessageCountError);
     }
 
     const messageList: ServiceBusReceivedMessage[] = [];

sdk/servicebus/service-bus/src/receivers/receiver.ts

@@ -15,10 +15,12 @@ import { ConnectionContext } from "../connectionContext";
 import {
   getAlreadyReceivingErrorMsg,
   getReceiverClosedErrorMsg,
+  InvalidMaxMessageCountError,
   throwErrorIfConnectionClosed,
   throwTypeErrorIfParameterMissing,
   throwTypeErrorIfParameterNotLong,
-  throwErrorIfInvalidOperationOnMessage
+  throwErrorIfInvalidOperationOnMessage,
+  throwTypeErrorIfParameterTypeMismatch
 } from "../util/errors";
 import { OnError, OnMessage, ReceiveOptions } from "../core/messageReceiver";
 import { StreamingReceiverInitArgs, StreamingReceiver } from "../core/streamingReceiver";
@@ -457,9 +459,20 @@ export class ServiceBusReceiverImpl implements ServiceBusReceiver {
   ): Promise<ServiceBusReceivedMessage[]> {
     this._throwIfReceiverOrConnectionClosed();
     this._throwIfAlreadyReceiving();
+    throwTypeErrorIfParameterMissing(
+      this._context.connectionId,
+      "maxMessageCount",
+      maxMessageCount
+    );
+    throwTypeErrorIfParameterTypeMismatch(
+      this._context.connectionId,
+      "maxMessageCount",
+      maxMessageCount,
+      "number"
+    );
 
-    if (maxMessageCount == undefined) {
-      maxMessageCount = 1;
+    if (isNaN(maxMessageCount) || maxMessageCount < 1) {
+      throw new TypeError(InvalidMaxMessageCountError);
     }
 
     const receiveMessages = async () => {
@@ -552,10 +565,6 @@ export class ServiceBusReceiverImpl implements ServiceBusReceiver {
   ): Promise<ServiceBusReceivedMessage[]> {
     this._throwIfReceiverOrConnectionClosed();
 
-    if (maxMessageCount == undefined) {
-      maxMessageCount = 1;
-    }
-
     const managementRequestOptions = {
       ...options,
       associatedLinkName: this._getAssociatedReceiverName(),

sdk/servicebus/service-bus/src/receivers/sessionReceiver.ts

@@ -8,10 +8,12 @@ import { MessageSession } from "../session/messageSession";
 import {
   getAlreadyReceivingErrorMsg,
   getReceiverClosedErrorMsg,
+  InvalidMaxMessageCountError,
   throwErrorIfConnectionClosed,
   throwTypeErrorIfParameterMissing,
   throwTypeErrorIfParameterNotLong,
-  throwErrorIfInvalidOperationOnMessage
+  throwErrorIfInvalidOperationOnMessage,
+  throwTypeErrorIfParameterTypeMismatch
 } from "../util/errors";
 import { OnError, OnMessage } from "../core/messageReceiver";
 import {
@@ -298,10 +300,6 @@ export class ServiceBusSessionReceiverImpl implements ServiceBusSessionReceiver
   ): Promise<ServiceBusReceivedMessage[]> {
     this._throwIfReceiverOrConnectionClosed();
 
-    if (maxMessageCount == undefined) {
-      maxMessageCount = 1;
-    }
-
     const managementRequestOptions = {
       ...options,
       associatedLinkName: this._messageSession.name,
@@ -381,9 +379,20 @@ export class ServiceBusSessionReceiverImpl implements ServiceBusSessionReceiver
   ): Promise<ServiceBusReceivedMessage[]> {
     this._throwIfReceiverOrConnectionClosed();
     this._throwIfAlreadyReceiving();
+    throwTypeErrorIfParameterMissing(
+      this._context.connectionId,
+      "maxMessageCount",
+      maxMessageCount
+    );
+    throwTypeErrorIfParameterTypeMismatch(
+      this._context.connectionId,
+      "maxMessageCount",
+      maxMessageCount,
+      "number"
+    );
 
-    if (maxMessageCount == undefined) {
-      maxMessageCount = 1;
+    if (isNaN(maxMessageCount) || maxMessageCount < 1) {
+      throw new TypeError(InvalidMaxMessageCountError);
     }
 
     const receiveBatchOperationPromise = async () => {

sdk/servicebus/service-bus/src/util/errors.ts

@@ -18,6 +18,14 @@ import { ReceiveMode } from "../models";
 export const entityPathMisMatchError =
   "The queue or topic name provided does not match the EntityPath in the connection string passed to the ServiceBusClient constructor.";
 
+/**
+ * Error message for when maxMessageCount provided is invalid.
+ *
+ * @internal
+ * @ignore
+ */
+export const InvalidMaxMessageCountError = "'maxMessageCount' must be a number greater than 0.";
+
 /**
  * @internal
  * @ignore
@@ -108,6 +116,7 @@ export function throwTypeErrorIfParameterMissing(
  * @param parameterValue Value of the parameter to type check
  * @param expectedType Expected type of the parameter
  */
+
 export function throwTypeErrorIfParameterTypeMismatch(
   connectionId: string,
   parameterName: string,

sdk/servicebus/service-bus/test/batchReceiver.spec.ts

@@ -52,74 +52,6 @@ function afterEachTest(): Promise<void> {
 }
 
 describe("Batching Receiver", () => {
-  describe("Batch Receiver - default values", function(): void {
-    before(() => {
-      serviceBusClient = createServiceBusClientForTests();
-    });
-
-    after(() => {
-      return serviceBusClient.test.after();
-    });
-
-    afterEach(async () => {
-      await afterEachTest();
-    });
-
-    it(noSessionTestClientType + ": maxMessageCount defaults to 1", async function(): Promise<
-      void
-    > {
-      await beforeEachTest(noSessionTestClientType);
-      const testMessage = TestMessage.getSample();
-      await sender.sendMessages(testMessage);
-
-      // @ts-expect-error
-      const peekedMsgs = await receiver.peekMessages();
-      should.equal(peekedMsgs.length, 1, "Unexpected number of messages peeked.");
-      should.equal(
-        peekedMsgs[0].body,
-        testMessage.body,
-        "Peeked message body is different than expected"
-      );
-
-      // @ts-expect-error
-      const msgs = await receiver.receiveMessages();
-      should.equal(msgs.length, 1, "Unexpected number of messages received.");
-      should.equal(
-        msgs[0].body,
-        testMessage.body,
-        "Received message body is different than expected"
-      );
-      await receiver.completeMessage(msgs[0]);
-    });
-
-    it(withSessionTestClientType + ": maxMessageCount defaults to 1", async function(): Promise<
-      void
-    > {
-      await beforeEachTest(withSessionTestClientType);
-      const testMessage = TestMessage.getSessionSample();
-      await sender.sendMessages(testMessage);
-
-      // @ts-expect-error
-      const peekedMsgs = await receiver.peekMessages();
-      should.equal(peekedMsgs.length, 1, "Unexpected number of messages peeked.");
-      should.equal(
-        peekedMsgs[0].body,
-        testMessage.body,
-        "Peeked message body is different than expected"
-      );
-
-      // @ts-expect-error
-      const msgs = await receiver.receiveMessages();
-      should.equal(msgs.length, 1, "Unexpected number of messages received.");
-      should.equal(
-        msgs[0].body,
-        testMessage.body,
-        "Received message body is different than expected"
-      );
-      await receiver.completeMessage(msgs[0]);
-    });
-  });
-
   describe("Batch Receiver - Settle message", function(): void {
     const maxDeliveryCount = 10;