Checkpoint

Author: bretambrose

lib/browser/mqtt_shared_browser.spec.ts

@@ -76,6 +76,7 @@ test('MQTT topic properties - invalid topic', async () => {
 test('metrics username construction - undefined username', async () => {
     let username = mqtt_shared_browser.buildFinalUsernameFromMetrics(new mqtt_shared.AwsIoTDeviceSDKMetrics());
     expect(username.startsWith(`?SDK=${mqtt_shared.SDK_NAME}&Platform=Browser`)).toBeTruthy();
+    expect(username.indexOf("&Metadata=(Browser=")).toBeGreaterThanOrEqual(0);
 });
 
 test('metrics username construction - empty username', async () => {
@@ -104,8 +105,8 @@ test('metrics username construction - query username with no overlap 3', async (
 });
 
 test('metrics username construction - query username with sdk overlap 1', async () => {
-    let username = mqtt_shared_browser.buildFinalUsernameFromMetrics(new mqtt_shared.AwsIoTDeviceSDKMetrics(), "hello?a=b&SDK=derp");
-    expect(username.startsWith(`hello?a=b&SDK=derp&Platform=Browser`)).toBeTruthy();
+    let username = mqtt_shared_browser.buildFinalUsernameFromMetrics(new mqtt_shared.AwsIoTDeviceSDKMetrics(), "hello?a=b&SDK");
+    expect(username.startsWith(`hello?a=b&SDK=&Platform=Browser`)).toBeTruthy();
 });
 
 test('metrics username construction - query username with sdk overlap 2', async () => {

lib/browser/mqtt_shared_browser.ts

@@ -175,34 +175,37 @@ export function calculateNextReconnectDelay(context: ReconnectDelayContext) : nu
 class ParsedUsername {
     prefix: string = "";
     queryParams: [string, string][] = new Array<[string, string]>;
-//    metadata: [string, string][] = new Array<[string, string]>;
+    metadata?: [string, string][] = undefined;
 }
 
 const METADATA_KEY : string = "Metadata";
 const SDK_KEY : string = "SDK";
 const PLATFORM_KEY : string = "Platform";
 const BROWSER_PLATFORM_VALUE : string = "Browser";
 const BROWSER_METADATA_KEY : string = "Browser";
-
-function parseDelimitedKeyValueString(input: string, pairDelimiter: string, kvDelimeter: string) : Array<[string, string]> | null {
+const QUERY_PARAM_START_DELIMITER : string = "?";
+const QUERY_PAIR_DELIMITER : string = "&";
+const METADATA_PAIR_DELIMITER : string = ";";
+const KEY_VALUE_SEPARATOR : string = "=";
+const METADATA_PREFIX : string = "(";
+const METADATA_SUFFIX : string = ")";
+
+function parseDelimitedKeyValueString(input: string, pairDelimiter: string, kvDelimiter: string) : Array<[string, string]> {
     let kvPairs = new Array<[string, string]>;
 
     let pairs = input.split(pairDelimiter);
     for (let pair of pairs) {
-        let kvDelimeterIndex = pair.indexOf(kvDelimeter);
-        if (kvDelimeterIndex < 0) {
+        let kvDelimiterIndex = pair.indexOf(kvDelimiter);
+        // just a key, no value?
+        if (kvDelimiterIndex < 0) {
             kvPairs.push([pair, ""]);
         }
 
-        let value = pair.substring(kvDelimeterIndex + 1);
-        kvPairs.push([pair.substring(0, kvDelimeterIndex), value]);
-    }
-
-    if (kvPairs.length > 0) {
-        return kvPairs;
+        let value = pair.substring(kvDelimiterIndex + 1);
+        kvPairs.push([pair.substring(0, kvDelimiterIndex), value]);
     }
 
-    return null;
+    return kvPairs;
 }
 
 function parseUsername(username?: string) : ParsedUsername {
@@ -212,80 +215,120 @@ function parseUsername(username?: string) : ParsedUsername {
         return parsed;
     }
 
-    let queryIndex = username.lastIndexOf('?');
+    let queryIndex = username.lastIndexOf(QUERY_PARAM_START_DELIMITER);
     if (queryIndex < 0) {
+        // no '?'
         parsed.prefix = username;
         return parsed;
     }
 
     parsed.prefix = username.substring(0, queryIndex);
 
     let remaining = username.substring(queryIndex + 1);
-    let topLevelPairs = parseDelimitedKeyValueString(remaining, "&", "=");
-    if (topLevelPairs) {
-        parsed.queryParams = topLevelPairs;
-        /*
-        let metadataValue = topLevelPairs.get(METADATA_KEY);
-        if (metadataValue !== undefined && metadataValue.length >= 2 && metadataValue[0] == '(' && metadataValue[metadataValue.length - 1] == ')') {
-            metadataValue = metadataValue.substring(1, metadataValue.length - 2);
-            let metadataMap = parseDelimitedKeyValueString(metadataValue, ";", "=");
-            if (metadataMap) {
-                parsed.metadata = metadataMap;
-                parsed.queryParams.delete(METADATA_KEY);
+    parsed.queryParams = parseDelimitedKeyValueString(remaining, QUERY_PAIR_DELIMITER, KEY_VALUE_SEPARATOR);
+    let seenMetadata = false;
+    for (let pair of parsed.queryParams) {
+        if (pair[0] === METADATA_KEY && !seenMetadata) {
+            // only process the first instance of metadata
+            seenMetadata = true;
+            let metadataValue = pair[1];
+            if (metadataValue !== undefined && metadataValue.length >= 2 && metadataValue.startsWith(METADATA_PREFIX) && metadataValue.endsWith(METADATA_SUFFIX)) {
+                metadataValue = metadataValue.substring(METADATA_PREFIX.length, metadataValue.length - (METADATA_PREFIX.length + METADATA_SUFFIX.length));
+                parsed.metadata = parseDelimitedKeyValueString(metadataValue, METADATA_PAIR_DELIMITER, KEY_VALUE_SEPARATOR);
             }
-        }*/
+            break;
+        }
     }
 
     return parsed;
 }
 
-function addTopLevelPairIfNonexistent(parsed: ParsedUsername, key: string, value: string) {
-    if (parsed.queryParams.has(key)) {
-        return;
+// if this is ever public, we need to handle attempting to set metadata in a special way
+function addTopLevelPair(parsed: ParsedUsername, key: string, value: string) {
+    // no need to check for duplicates; when we build the final username we use the first value only
+    parsed.queryParams.push([key, value]);
+}
+
+function addMetadataPair(parsed: ParsedUsername, key: string, value: string) {
+    let hasMetadataKey = false;
+    for (let pair of parsed.queryParams) {
+        if (pair[0] === METADATA_KEY) {
+            hasMetadataKey = true;
+            break;
+        }
     }
 
-    parsed.queryParams.set(key, value);
-}
+    // If we have malformed metadata (top level entry, but no parsed pairs), then don't do anything
+    if (!parsed.metadata) {
+        if (hasMetadataKey) {
+            return;
+        }
 
-function addMetadataPairIfNonExistent(parsed: ParsedUsername, key: string, value: string) {
-    // can't add metadata pairs if there's an existing top-level metadata entry that is malformed
-    if (parsed.queryParams.has(METADATA_KEY)) {
-        return;
+        parsed.metadata = new Array<[string, string]>();
     }
 
-    let metadataValue = parsed.metadata.get(key);
-    if (metadataValue !== undefined) {
-        return;
+    // no need to check for duplicates; when we build the final username we use the first value only
+    let strippedValue = value.replace(METADATA_PAIR_DELIMITER, "");
+    parsed.metadata.push([key, strippedValue]);
+}
+
+function buildOrderedKeyValues(pairs?: Array<[string, string]>) : [Array<string>, Map<string, string>] {
+    let keys: Array<string> = new Array<string>();
+    let kvMap = new Map<string, string>();
+    if (!pairs) {
+        return [keys, kvMap];
     }
 
-    parsed.metadata.set(key, value);
+    pairs.forEach((pair) => {
+       if (!kvMap.has(pair[0])) {
+           kvMap.set(pair[0], pair[1]);
+           keys.push(pair[0]);
+       }
+    });
+
+    return [keys, kvMap];
 }
 
 function buildUsernameFromQueryParse(parsed: ParsedUsername) : string {
-    let metadataValue = undefined;
-    if (!parsed.queryParams.get(METADATA_KEY) && parsed.metadata.size > 0) {
-        let innerValue = Array.from(parsed.metadata.entries()).map((pair) => pair.join("=")).join(";");
-        metadataValue = "(" + innerValue + ")";
+    let [queryKeys, queryValues] = buildOrderedKeyValues(parsed.queryParams);
+    let [metadataKeys, metadataValues] = buildOrderedKeyValues(parsed.metadata);
+
+    if (parsed.queryParams.length == 0 && !parsed.metadata) {
+        return parsed.prefix;
+    }
+
+    // if we have valid metadata pairs, build the final metadata value
+    let metadataValue : string | undefined = undefined;
+    if (parsed.metadata && metadataKeys.length > 0) {
+        let innerValue = metadataKeys.map((key) => {
+            return `${key}${KEY_VALUE_SEPARATOR}${metadataValues.get(key) ?? ""}`;
+        }).join(METADATA_PAIR_DELIMITER);
+        metadataValue = METADATA_PREFIX + innerValue + METADATA_SUFFIX;
     }
 
-    let topLevelParamArray = Array.from(parsed.queryParams.entries());
+    // if we have a constructed metadata value, replace the value and make sure it's in the key list
     if (metadataValue !== undefined) {
-        topLevelParamArray.push([METADATA_KEY, metadataValue]);
+        if (queryValues.get(METADATA_KEY) === undefined) {
+            queryKeys.push(METADATA_KEY);
+        }
+        queryValues.set(METADATA_KEY, metadataValue);
     }
 
-    let queryParamValue = topLevelParamArray.map((pair) => pair.join("=")).join(":");
+    let queryParamValue = queryKeys.map((key) => {
+        return `${key}${KEY_VALUE_SEPARATOR}${queryValues.get(key) ?? ""}`;
+    }).join(QUERY_PAIR_DELIMITER);
 
-    return parsed.prefix + "?" + queryParamValue;
+    return parsed.prefix + QUERY_PARAM_START_DELIMITER + queryParamValue;
 }
 
 export function buildFinalUsernameFromMetrics(metrics: mqtt_shared.AwsIoTDeviceSDKMetrics, username?: string) : string {
     let parsed = parseUsername(username);
 
-    addTopLevelPairIfNonexistent(parsed, SDK_KEY, metrics.libraryName);
-    addTopLevelPairIfNonexistent(parsed, PLATFORM_KEY, BROWSER_PLATFORM_VALUE);
+    addTopLevelPair(parsed, SDK_KEY, metrics.libraryName);
+    addTopLevelPair(parsed, PLATFORM_KEY, BROWSER_PLATFORM_VALUE);
 
     let browserInfo = window?.navigator?.userAgent ?? "unknown";
-    addMetadataPairIfNonExistent(parsed, BROWSER_METADATA_KEY, browserInfo);
+    addMetadataPair(parsed, BROWSER_METADATA_KEY, browserInfo);
 
     return buildUsernameFromQueryParse(parsed);
 }