[Core AMQP] Do not stringify entityPath for undefined (Azure#12321)

Fixes Azure#8105 and Azure#11155

Author: HarshaNalluru

sdk/core/core-amqp/CHANGELOG.md

@@ -2,6 +2,11 @@
 
 ## 2.0.0 (Unreleased)
 
+### Breaking Changes
+
+- Previously, `ConnectionConfig.validate()` overridden entityPath if `undefined` with `String(undefined) = "undefined"`. This has been updated to retain `undefined` in the validation.
+  [PR 12321](https://github.com/Azure/azure-sdk-for-js/pull/12321)
+
 ## 2.0.0-beta.1 (2020-11-03)
 
 - `AmqpAnnotatedMessage` interface that closely represents the AMQP annotated message from the [AMQP spec](https://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-messaging-v1.0-os.html#section-message-format) has been added. New `AmqpMessageHeaders` and `AmqpMessageProperties` interfaces(properties with camelCasing) have been added in the place of re-exports from "rhea" library(properties with snake_casing).

sdk/core/core-amqp/package.json

@@ -51,9 +51,9 @@
     "lint": "eslint package.json api-extractor.json src test --ext .ts -f html -o core-amqp-lintReport.html || exit 0",
     "pack": "npm pack 2>&1",
     "prebuild": "npm run clean",
-    "test:browser": "npm run build:test && npm run unit-test:browser && npm run integration-test:browser",
-    "test:node": "npm run build:test && npm run unit-test:node && npm run integration-test:node",
-    "test": "npm run build:test && npm run unit-test && npm run integration-test",
+    "test:browser": "npm run clean && npm run build:test && npm run unit-test:browser",
+    "test:node": "npm run clean && npm run build:test && npm run unit-test:node",
+    "test": "npm run test:node && npm run test:browser",
     "unit-test:browser": "karma start --single-run",
     "unit-test:node": "cross-env TS_NODE_FILES=true TS_NODE_COMPILER_OPTIONS=\"{\\\"module\\\":\\\"commonjs\\\"}\" nyc --reporter=lcov --reporter=text-lcov mocha -r ts-node/register -t 50000 --reporter ../../../common/tools/mocha-multi-reporter.js ./test/**/*.spec.ts",
     "unit-test": "npm run unit-test:node && npm run unit-test:browser"

sdk/core/core-amqp/src/connectionConfig/connectionConfig.ts

@@ -134,7 +134,9 @@ export const ConnectionConfig = {
     if (options.isEntityPathRequired && !config.entityPath) {
       throw new TypeError("Missing 'entityPath' in configuration");
     }
-    config.entityPath = String(config.entityPath);
+    if (config.entityPath != undefined) {
+       config.entityPath = String(config.entityPath);
+    }
 
     if (!isSharedAccessSignature(config.connectionString)) {
       if (!config.sharedAccessKeyName) {

sdk/core/core-amqp/test/config.spec.ts

@@ -194,6 +194,46 @@ describe("ConnectionConfig", function() {
         done();
       });
     });
+
+    describe("EntityPath Validation", function() {
+      const connectionString = `
+        Endpoint=sb://hostname.servicebus.windows.net/;
+        SharedAccessKeyName=sakName;
+        SharedAccessKey=sakName;
+        EntityPath=ep;
+      `;
+      const config: ConnectionConfig = {
+        connectionString: connectionString,
+        endpoint: "sb://hostname.servicebus.windows.net/",
+        host: "hostname.servicebus.windows.net/",
+        sharedAccessKeyName: "sakName",
+        sharedAccessKey: "abcd"
+      };
+
+      it("undefined is not stringified", () => {
+        config.entityPath = undefined;
+        ConnectionConfig.validate(config);
+        should.equal(config.entityPath, undefined, `EntityPath is not undefined`);
+      });
+
+      it("null is not stringified", () => {
+        config.entityPath = null as any;
+        ConnectionConfig.validate(config);
+        should.equal(config.entityPath, null, `EntityPath is not null`);
+      });
+
+      it("number is stringified", () => {
+        config.entityPath = 3 as any;
+        ConnectionConfig.validate(config);
+        should.equal(config.entityPath, "3", `EntityPath is not stringified`);
+      });
+
+      it("string is unchanged", () => {
+        config.entityPath = "entityPath";
+        ConnectionConfig.validate(config);
+        should.equal(config.entityPath, "entityPath", `EntityPath is not a string`);
+      });
+    });
   });
 
   describe("SharedAccessSignature", () => {

sdk/servicebus/service-bus/src/constructorHelpers.ts

@@ -26,21 +26,6 @@ export interface ServiceBusClientOptions {
   userAgentOptions?: UserAgentOptions;
 }
 
-/**
- * @internal
- * @ignore
- *
- * @param {ConnectionConfig} config
- */
-function validate(config: ConnectionConfig) {
-  // TODO: workaround - core-amqp's validate string-izes "undefined"
-  // the timing of this particular call happens in a spot where we might not have an
-  // entity path so it's perfectly legitimate for it to be empty.
-  config.entityPath = config.entityPath ?? "";
-
-  ConnectionConfig.validate(config);
-}
-
 /**
  * @internal
  * @ignore
@@ -60,7 +45,6 @@ export function createConnectionContext(
   config.webSocketEndpointPath = "$servicebus/websocket";
   config.webSocketConstructorOptions = options?.webSocketOptions?.webSocketConstructorOptions;
 
-  validate(config);
   return ConnectionContext.create(config, credential, options);
 }
 

sdk/servicebus/service-bus/test/internal/serviceBusClient.spec.ts

@@ -51,7 +51,10 @@ describe("serviceBusClient unit tests", () => {
 
         client["_connectionContext"] = createConnectionContextForTestsWithSessionId(
           "a session id",
-          origConnectionContext.config
+          {
+            ...origConnectionContext.config,
+            entityPath: testEntity.topic ? testEntity.topic : testEntity.queue
+          }
         );
 
         let sessionReceiver: ServiceBusSessionReceiver;
@@ -102,10 +105,10 @@ describe("serviceBusClient unit tests", () => {
 
         const origConnectionContext = client["_connectionContext"];
 
-        client["_connectionContext"] = createConnectionContextForTestsWithSessionId(
-          "session id",
-          origConnectionContext.config
-        );
+        client["_connectionContext"] = createConnectionContextForTestsWithSessionId("session id", {
+          ...origConnectionContext.config,
+          entityPath: testEntity.topic ? testEntity.topic : testEntity.queue
+        });
 
         let sessionReceiver: ServiceBusSessionReceiver;
 
@@ -317,16 +320,6 @@ describe("serviceBusClient unit tests", () => {
         });
         validateWebsocketInfo(connectionContext, options);
       });
-
-      it("undefined entity path is translated to ''", () => {
-        const connString = "Endpoint=sb://a;SharedAccessKeyName=b;SharedAccessKey=c;";
-        const connectionContext = createConnectionContextForConnectionString(connString, {});
-        assert.equal(
-          connectionContext.config.entityPath,
-          "",
-          "Unexpected entityPath in the connection config"
-        );
-      });
     });
 
     describe("createConnectionContextForTokenCredential", () => {
@@ -345,19 +338,6 @@ describe("serviceBusClient unit tests", () => {
         );
         validateWebsocketInfo(connectionContext, options);
       });
-
-      it("undefined entity path is translated to ''", () => {
-        const connectionContext = createConnectionContextForTokenCredential(
-          pseudoTokenCred,
-          endpoint,
-          {}
-        );
-        assert.equal(
-          connectionContext.config.entityPath,
-          "",
-          "Unexpected entityPath in the connection config"
-        );
-      });
     });
   });
 });