[Service Bus] Bug Fix: userProperties aren't populated while using cr…

…eateRule() from ATOM API (Azure#9794) * fix ruleResourceSerializer w.r.t userProperties * add test for userProperties * changelog * update error messages * remove console.log * Update sdk/servicebus/service-bus/CHANGELOG.md Co-authored-by: Ramya Rao <ramya.rao.a@outlook.com> * remove extra types * Update enum names to TypeMaps * fix the 3 failed tests * keyValueTypeXMLTag * buildRawSqlParameter -> buildRawKeyValuePairFromSqlParameter * more docs and renames * Add test for array * InternalRawKeyValuePairs, refactoring and much more * scope the test to user-properties * refactor parameter.type * update error message * remove .only * fix error * unneeded .valueOf * Add boolean type * Validations * tests for Type validation errors on Correlation user property inputs * Update sdk/servicebus/service-bus/src/serializers/ruleResourceSerializer.ts * Update sdk/servicebus/service-bus/src/serializers/ruleResourceSerializer.ts * fix test failures Co-authored-by: Ramya Rao <ramya.rao.a@outlook.com>
AzureSDKAutomation · Jul 2, 2020 · a7de6ea · a7de6ea
1 parent b0119f9
commit a7de6ea
Show file tree

Hide file tree

Showing 5 changed files with 345 additions and 66 deletions.
diff --git a/sdk/servicebus/service-bus/CHANGELOG.md b/sdk/servicebus/service-bus/CHANGELOG.md
@@ -5,10 +5,10 @@
 - Adds abortSignal support throughout Sender and non-session Receivers.
   [PR 9233](https://github.com/Azure/azure-sdk-for-js/pull/9233)
   [PR 9284](https://github.com/Azure/azure-sdk-for-js/pull/9284)
-- Standardized methods on senders and receivers to use the `Messages` suffix and deal with multiple messages rather than
-have dedicated methods to deal with a single message. [PR 9678](https://github.com/Azure/azure-sdk-for-js/pull/9678)
-- Standardized methods that peek and receive given number messages to use similar signature. 
-[PR 9798](https://github.com/Azure/azure-sdk-for-js/pull/9798)
+- Standardized methods on senders and receivers to use the `Messages` suffix and deal with multiple messages rather than have dedicated methods to deal with a single message. 
+  [PR 9678](https://github.com/Azure/azure-sdk-for-js/pull/9678)
+- Standardized methods that peek and receive given number messages to use similar signature.
+  [PR 9798](https://github.com/Azure/azure-sdk-for-js/pull/9798)
 - Bug - Messages scheduled in parallel with the `scheduleMessage` method have the same sequence number in response.
   Fixed in [PR 9503](https://github.com/Azure/azure-sdk-for-js/pull/9503)
 - Management api updates (Includes breaking changes)
@@ -27,6 +27,8 @@ have dedicated methods to deal with a single message. [PR 9678](https://github.c
 
     See [update queue](https://docs.microsoft.com/en-us/rest/api/servicebus/update-queue) and [update-topic](https://docs.microsoft.com/en-us/rest/api/servicebus/update-queue) for list of updatable properties.
 
+  - Fixed the bug where one cannot set `userProperties` in a correlation filter while using the `createRule()` method. [PR 9794](https://github.com/Azure/azure-sdk-for-js/pull/9794)
+
 ## 7.0.0-preview.3 (2020-06-08)
 
 - Improves the performance of the `ServiceBusMessageBatch.tryAdd` method.

diff --git a/sdk/servicebus/service-bus/src/serializers/ruleResourceSerializer.ts b/sdk/servicebus/service-bus/src/serializers/ruleResourceSerializer.ts
@@ -59,7 +59,7 @@ function getTopicFilter(value: any): SqlRuleFilter | CorrelationRuleFilter {
       sessionId: getStringOrUndefined(value["SessionId"]),
       messageId: getStringOrUndefined(value["MessageId"]),
       contentType: getStringOrUndefined(value["ContentType"]),
-      userProperties: value["UserProperties"]
+      userProperties: getUserPropertiesOrUndefined(value["Properties"])
     };
   }
   return result;
@@ -188,7 +188,7 @@ export class RuleResourceSerializer implements AtomXmlSerializer {
           ContentType: correlationFilter.contentType,
           SessionId: correlationFilter.sessionId,
           MessageId: correlationFilter.messageId,
-          Properties: correlationFilter.userProperties
+          Properties: getRawUserProperties(correlationFilter.userProperties)
         };
         resource.Filter[Constants.XML_METADATA_MARKER] = {
           "p4:type": "CorrelationFilter",
@@ -226,16 +226,30 @@ export class RuleResourceSerializer implements AtomXmlSerializer {
 }
 
 /**
+ * Service expects the XML request with the special type names serialized in the request,
+ * the request would fail otherwise.
+ *
  * @internal
  * @ignore
- * @enum {number}
  */
-enum SqlParameterType {
-  Integer = "l28:int",
-  String = "l28:string",
-  Long = "l28:long",
-  Date = "l28:date"
-}
+const TypeMapForRequestSerialization: Record<string, string> = {
+  int: "l28:int",
+  string: "l28:string",
+  long: "l28:long",
+  date: "l28:date",
+  boolean: "l28:boolean"
+};
+
+/**
+ * @internal
+ * @ignore
+ */
+const TypeMapForResponseDeserialization: Record<string, string> = {
+  number: "d6p1:int",
+  string: "d6p1:string",
+  boolean: "d6p1:boolean"
+};
+
 /**
  * Represents type of SQL `Parameter` in ATOM based management operations
  */
@@ -248,13 +262,25 @@ export type SqlParameter = {
 /**
  * @internal
  * @ignore
- * Internal representation of SQL parameter info
+ * Internal representation of key-value pair
  */
-type RawSqlParameter = {
+type RawKeyValuePair = {
   Key: string;
   Value: any;
 };
 
+interface InternalRawKeyValuePairs {
+  KeyValueOfstringanyType: RawKeyValuePair[];
+}
+
+/**
+ * Key-value pairs are supposed to be wrapped with this tag in the XML request, they are ignored otherwise.
+ *
+ * @internal
+ * @ignore
+ */
+const keyValuePairXMLTag = "KeyValueOfstringanyType";
+
 /**
  * @internal
  * @ignore
@@ -274,7 +300,7 @@ function getSqlParametersOrUndefined(value: any): SqlParameter[] | undefined {
     return undefined;
   }
 
-  const rawParameters = value["KeyValueOfstringanyType"];
+  const rawParameters = value[keyValuePairXMLTag];
   if (Array.isArray(rawParameters)) {
     for (let i = 0; i < rawParameters.length; i++) {
       parameters.push(buildSqlParameter(rawParameters[i]));
@@ -288,11 +314,49 @@ function getSqlParametersOrUndefined(value: any): SqlParameter[] | undefined {
 /**
  * @internal
  * @ignore
- * Helper utility to build an instance of parsed SQL parameteras `Parameter`
+ * Helper utility to retrieve the user-properties from given input,
+ * or undefined if not passed in.
+ * @param value
+ */
+function getUserPropertiesOrUndefined(value: any): { [key: string]: any } | undefined {
+  if (!value) {
+    return undefined;
+  }
+  const properties: any = {};
+  const rawProperties = value[keyValuePairXMLTag];
+  if (Array.isArray(rawProperties)) {
+    for (const rawProperty of rawProperties) {
+      properties[rawProperty.Key] = rawProperty.Value["_"];
+      if (rawProperty.Value["$"]["i:type"] === TypeMapForResponseDeserialization.number) {
+        properties[rawProperty.Key] = Number(rawProperty.Value["_"]);
+      } else if (rawProperty.Value["$"]["i:type"] === TypeMapForResponseDeserialization.string) {
+        properties[rawProperty.Key] = rawProperty.Value["_"];
+      } else if (rawProperty.Value["$"]["i:type"] === TypeMapForResponseDeserialization.boolean) {
+        properties[rawProperty.Key] = rawProperty.Value["_"] === "true" ? true : false;
+      } else {
+        throw new TypeError(
+          `Unable to parse the user property in the response - ${JSON.stringify(rawProperty)}`
+        );
+      }
+    }
+  } else {
+    throw new TypeError(
+      `"UserProperties" in the response is not an array, unable to parse the response - ${JSON.stringify(
+        value
+      )}`
+    );
+  }
+  return properties;
+}
+
+/**
+ * @internal
+ * @ignore
+ * Helper utility to build an instance of parsed SQL parameters `Parameter`
  * from given input
  * @param value
  */
-function buildSqlParameter(value: RawSqlParameter): SqlParameter {
+function buildSqlParameter(value: RawKeyValuePair): SqlParameter {
   const rawValue = value["Value"]["_"];
   const type = value["Value"]["$"]["i:type"].toString().substring(5);
   let parsedValue: any;
@@ -307,7 +371,7 @@ function buildSqlParameter(value: RawSqlParameter): SqlParameter {
       break;
 
     default:
-      throw new Error(
+      throw new TypeError(
         `Invalid type "${type}" on the SQL Parameter. Must be either of "interface, "string", "long" or "date".`
       );
   }
@@ -326,7 +390,9 @@ function buildSqlParameter(value: RawSqlParameter): SqlParameter {
  * or undefined if not passed in.
  * @param value
  */
-export function getRawSqlParameters(parameters: SqlParameter[] | undefined): any {
+export function getRawSqlParameters(
+  parameters: SqlParameter[] | undefined
+): InternalRawKeyValuePairs | undefined {
   if (parameters == undefined) {
     return undefined;
   }
@@ -341,21 +407,78 @@ export function getRawSqlParameters(parameters: SqlParameter[] | undefined): any
     );
   }
 
-  const rawParameters: RawSqlParameter[] = [];
+  const rawParameters: RawKeyValuePair[] = [];
   for (let i = 0; i < parameters.length; i++) {
-    rawParameters.push(buildRawSqlParameter(parameters[i]));
+    rawParameters.push(buildRawKeyValuePairFromSqlParameter(parameters[i]));
+  }
+  return { [keyValuePairXMLTag]: rawParameters };
+}
+
+/**
+ * @internal
+ * @ignore
+ * Helper utility to extract array of userProperties key-value instances from given input,
+ * or undefined if not passed in.
+ * @param value
+ */
+export function getRawUserProperties(
+  parameters: { [key: string]: any } | undefined
+): InternalRawKeyValuePairs | undefined {
+  if (parameters == undefined) {
+    return undefined;
+  }
+  if (
+    Array.isArray(parameters) ||
+    typeof parameters === "string" ||
+    typeof parameters !== "object" ||
+    Object.entries(parameters).length < 1
+  ) {
+    throw new TypeError(
+      `Unsupported value for the userProperties ${JSON.stringify(
+        parameters
+      )}, expected a JSON object with key-value pairs.`
+    );
+  }
+  const rawParameters: RawKeyValuePair[] = [];
+  for (let [key, value] of Object.entries(parameters)) {
+    let type: string | number | boolean;
+    if (typeof value === "number") {
+      type = TypeMapForRequestSerialization.int;
+    } else if (typeof value === "string") {
+      type = TypeMapForRequestSerialization.string;
+    } else if (typeof value === "boolean") {
+      type = TypeMapForRequestSerialization.boolean;
+    } else {
+      throw new TypeError(
+        `Unsupported type for the value in the user property {${key}:${JSON.stringify(value)}}`
+      );
+    }
+
+    const rawParameter: RawKeyValuePair = {
+      Key: key,
+      Value: {
+        [Constants.XML_METADATA_MARKER]: {
+          "p4:type": type,
+          "xmlns:l28": "http://www.w3.org/2001/XMLSchema"
+        },
+        [Constants.XML_VALUE_MARKER]: value
+      }
+    };
+    rawParameters.push(rawParameter);
   }
-  return { KeyValueOfstringanyType: rawParameters };
+  return {
+    [keyValuePairXMLTag]: rawParameters
+  };
 }
 
 /**
  * @internal
  * @ignore
- * Helper utility to build an instance of raw SQL parameter as `RawSqlParameter`
+ * Helper utility to build an instance of raw SQL parameter as `RawKeyValuePair`
  * from given `SqlParameter` input,
  * @param parameter parsed SQL parameter instance
  */
-function buildRawSqlParameter(parameter: SqlParameter): RawSqlParameter {
+function buildRawKeyValuePairFromSqlParameter(parameter: SqlParameter): RawKeyValuePair {
   if (!isJSONLikeObject(parameter) || parameter === null) {
     throw new TypeError(
       `Expected SQL parameter input to be a JS object value, but received ${JSON.stringify(
@@ -366,37 +489,22 @@ function buildRawSqlParameter(parameter: SqlParameter): RawSqlParameter {
     );
   }
 
-  let type: SqlParameterType;
-  switch (parameter.type) {
-    case "int":
-      type = SqlParameterType.Integer;
-      break;
-    case "string":
-      type = SqlParameterType.String;
-      break;
-    case "long":
-      type = SqlParameterType.Long;
-      break;
-    case "date":
-      type = SqlParameterType.Date;
-      break;
-
-    default:
-      throw new Error(
-        `Invalid type "${parameter.type}" supplied for the SQL Parameter. Must be either of "interface, "string", "long" or "date".`
-      );
+  let paramType = TypeMapForRequestSerialization[parameter.type];
+  if (!paramType) {
+    throw new Error(
+      `Invalid type "${parameter.type}" supplied for the SQL Parameter. Must be either of "int", "string", "long" or "date".`
+    );
   }
 
-  const rawParameterValue: any = {};
-  rawParameterValue[Constants.XML_METADATA_MARKER] = {
-    "p4:type": type.valueOf(),
-    "xmlns:l28": "http://www.w3.org/2001/XMLSchema"
-  };
-  rawParameterValue[Constants.XML_VALUE_MARKER] = parameter.value;
-
-  const rawParameter: RawSqlParameter = {
+  const rawParameter: RawKeyValuePair = {
     Key: parameter.key,
-    Value: rawParameterValue
+    Value: {
+      [Constants.XML_METADATA_MARKER]: {
+        "p4:type": paramType,
+        "xmlns:l28": "http://www.w3.org/2001/XMLSchema"
+      },
+      [Constants.XML_VALUE_MARKER]: parameter.value
+    }
   };
   return rawParameter;
 }
diff --git a/sdk/servicebus/service-bus/test/atomManagement.spec.ts b/sdk/servicebus/service-bus/test/atomManagement.spec.ts
@@ -1493,7 +1493,6 @@ describe("Atom management - Authentication", function(): void {
         sqlParameters: undefined,
         compatibilityLevel: undefined
       },
-
       name: managementRule1
     }
   },
@@ -1525,15 +1524,20 @@ describe("Atom management - Authentication", function(): void {
         sqlParameters: undefined,
         compatibilityLevel: 20
       },
-
       name: managementRule1
     }
   },
   {
     testCaseTitle: "Correlation Filter rule options",
     input: {
       filter: {
-        correlationId: "abcd"
+        correlationId: "abcd",
+        userProperties: {
+          randomState: "WA",
+          randomCountry: "US",
+          randomCount: 25,
+          randomBool: true
+        }
       },
       action: { sqlExpression: "SET sys.label='GREEN'" }
     },
@@ -1547,15 +1551,19 @@ describe("Atom management - Authentication", function(): void {
         replyToSessionId: "",
         sessionId: "",
         to: "",
-        userProperties: undefined
+        userProperties: {
+          randomState: "WA",
+          randomCountry: "US",
+          randomCount: 25,
+          randomBool: true
+        }
       },
       action: {
         sqlExpression: "SET sys.label='GREEN'",
         requiresPreprocessing: false,
         sqlParameters: undefined,
         compatibilityLevel: 20
       },
-
       name: managementRule1
     }
   }
@@ -1581,7 +1589,6 @@ describe("Atom management - Authentication", function(): void {
         undefined,
         testCase.input
       );
-
       should.equal(response.name, managementRule1, "Rule name mismatch");
       assert.deepEqualExcluding(response, testCase.output, [
         "_response",