[Telemetry API] Prevent Subscriptions with different options from overwriting each other (#7930)

* initial implementation

* cleaning up a bit

* adding the hash method back as we dont want gigantic keys

* adding a line

* added filtering to state generator, updated filters readme to fix error, more robust hash function

* removing unnecessary changes in wrong file

* adding a test to confirm each endpoint has a separate subscription based of filtering

* lint

* adding back in hints, accidentally removed

* remove some redundant code and convert sanitization method into a replacer function for stringify

* tweaking serialize replacer to handle arrays correctly, adding more determinative row addition check to test

* more focused selector for the table

* simplified the serialization method even further and added some more docs

Author: jvigliotta

e2e/tests/functional/plugins/displayLayout/displayLayout.e2e.spec.js

@@ -507,8 +507,153 @@ test.describe('Display Layout', () => {
     // In real time mode, we don't fetch annotations at all
     await expect.poll(() => networkRequests, { timeout: 10000 }).toHaveLength(0);
   });
+
+  test('Same objects with different request options have unique subscriptions', async ({
+    page
+  }) => {
+    // Expand My Items
+    await page.getByLabel('Expand My Items folder').click();
+
+    // Create a Display Layout
+    const displayLayout = await createDomainObjectWithDefaults(page, {
+      type: 'Display Layout',
+      name: 'Test Display'
+    });
+
+    // Create a State Generator, set to higher frequency updates
+    const stateGenerator = await createDomainObjectWithDefaults(page, {
+      type: 'State Generator',
+      name: 'State Generator'
+    });
+    const stateGeneratorTreeItem = page.getByRole('treeitem', {
+      name: stateGenerator.name
+    });
+    await stateGeneratorTreeItem.click({ button: 'right' });
+    await page.getByLabel('Edit Properties...').click();
+    await page.getByLabel('State Duration (seconds)', { exact: true }).fill('0.1');
+    await page.getByLabel('Save').click();
+
+    // Create a Table for filtering ON values
+    const tableFilterOnValue = await createDomainObjectWithDefaults(page, {
+      type: 'Telemetry Table',
+      name: 'Table Filter On Value'
+    });
+    const tableFilterOnTreeItem = page.getByRole('treeitem', {
+      name: tableFilterOnValue.name
+    });
+
+    // Create a Table for filtering OFF values
+    const tableFilterOffValue = await createDomainObjectWithDefaults(page, {
+      type: 'Telemetry Table',
+      name: 'Table Filter Off Value'
+    });
+    const tableFilterOffTreeItem = page.getByRole('treeitem', {
+      name: tableFilterOffValue.name
+    });
+
+    // Navigate to ON filtering table and add state generator and setup filters
+    await page.goto(tableFilterOnValue.url);
+    await stateGeneratorTreeItem.dragTo(page.getByLabel('Object View'));
+    await selectFilterOption(page, '1');
+    await page.getByLabel('Save').click();
+    await page.getByRole('listitem', { name: 'Save and Finish Editing' }).click();
+
+    // Navigate to OFF filtering table and add state generator and setup filters
+    await page.goto(tableFilterOffValue.url);
+    await stateGeneratorTreeItem.dragTo(page.getByLabel('Object View'));
+    await selectFilterOption(page, '0');
+    await page.getByLabel('Save').click();
+    await page.getByRole('listitem', { name: 'Save and Finish Editing' }).click();
+
+    // Navigate to the display layout and edit it
+    await page.goto(displayLayout.url);
+
+    // Add the tables to the display layout
+    await page.getByLabel('Edit Object').click();
+    await tableFilterOffTreeItem.dragTo(page.getByLabel('Layout Grid'), {
+      targetPosition: { x: 10, y: 300 }
+    });
+    await page.locator('.c-frame-edit > div:nth-child(4)').dragTo(page.getByLabel('Layout Grid'), {
+      targetPosition: { x: 400, y: 500 },
+      // eslint-disable-next-line playwright/no-force-option
+      force: true
+    });
+    await tableFilterOnTreeItem.dragTo(page.getByLabel('Layout Grid'), {
+      targetPosition: { x: 10, y: 100 }
+    });
+    await page.locator('.c-frame-edit > div:nth-child(4)').dragTo(page.getByLabel('Layout Grid'), {
+      targetPosition: { x: 400, y: 300 },
+      // eslint-disable-next-line playwright/no-force-option
+      force: true
+    });
+    await page.getByLabel('Save').click();
+    await page.getByRole('listitem', { name: 'Save and Finish Editing' }).click();
+
+    // Get the tables so we can verify filtering is working as expected
+    const tableFilterOn = page.getByLabel(`${tableFilterOnValue.name} Frame`, {
+      exact: true
+    });
+    const tableFilterOff = page.getByLabel(`${tableFilterOffValue.name} Frame`, {
+      exact: true
+    });
+
+    // Verify filtering is working correctly
+
+    // Create a promise that resolves when we've seen enough new rows added
+    const rowsMutationPromise = page.evaluate(() => {
+      return new Promise((resolve) => {
+        const targetTable = document.querySelector(
+          'table[aria-label="Table Filter Off Value table content"]'
+        );
+        const config = { childList: true, subtree: true };
+        let changeCount = 0;
+        const requiredChanges = 20; // Number of changes to wait for
+
+        const observer = new MutationObserver((mutations) => {
+          mutations.forEach((mutation) => {
+            if (mutation.type === 'childList') {
+              // Count added nodes
+              changeCount += mutation.addedNodes.length;
+            }
+          });
+
+          // Check if the required number of changes has been met
+          if (changeCount >= requiredChanges) {
+            observer.disconnect(); // Disconnect observer after the required changes
+            resolve();
+          }
+        });
+
+        observer.observe(targetTable, config);
+      });
+    });
+
+    await rowsMutationPromise;
+
+    // Check ON table doesn't have any OFF values
+    await expect(tableFilterOn.locator('td[title="OFF"]')).toHaveCount(0);
+    const onCount = await tableFilterOn.locator('td[title="ON"]').count();
+    await expect(onCount).toBeGreaterThan(0);
+
+    // Check OFF table doesn't have any ON values
+    await expect(tableFilterOff.locator('td[title="ON"]')).toHaveCount(0);
+    const offCount = await tableFilterOff.locator('td[title="OFF"]').count();
+    await expect(offCount).toBeGreaterThan(0);
+  });
 });
 
+async function selectFilterOption(page, filterOption) {
+  await page.getByRole('tab', { name: 'Filters' }).click();
+  await page
+    .getByLabel('Inspector Views')
+    .locator('li')
+    .filter({ hasText: 'State Generator' })
+    .locator('span')
+    .click();
+  await page.getByRole('switch').click();
+  await page.selectOption('select[name="setSelectionThreshold"]', filterOption);
+}
+
 async function addAndRemoveDrawingObjectAndAssert(page, layoutObject, DISPLAY_LAYOUT_NAME) {
   await expect(page.getByLabel(layoutObject, { exact: true })).toHaveCount(0);
   await addLayoutObject(page, DISPLAY_LAYOUT_NAME, layoutObject);

example/generator/GeneratorMetadataProvider.js

@@ -108,6 +108,16 @@ const METADATA_BY_TYPE = {
             string: 'ON'
           }
         ],
+        filters: [
+          {
+            singleSelectionThreshold: true,
+            comparator: 'equals',
+            possibleValues: [
+              { label: 'OFF', value: 0 },
+              { label: 'ON', value: 1 }
+            ]
+          }
+        ],
         hints: {
           range: 1
         }

example/generator/StateGeneratorProvider.js

@@ -34,14 +34,16 @@ StateGeneratorProvider.prototype.supportsSubscribe = function (domainObject) {
   return domainObject.type === 'example.state-generator';
 };
 
-StateGeneratorProvider.prototype.subscribe = function (domainObject, callback) {
+StateGeneratorProvider.prototype.subscribe = function (domainObject, callback, options) {
   var duration = domainObject.telemetry.duration * 1000;
 
-  var interval = setInterval(function () {
+  var interval = setInterval(() => {
     var now = Date.now();
     var datum = pointForTimestamp(now, duration, domainObject.name);
-    datum.value = String(datum.value);
-    callback(datum);
+    if (!this.shouldBeFiltered(datum, options)) {
+      datum.value = String(datum.value);
+      callback(datum);
+    }
   }, duration);
 
   return function () {
@@ -63,9 +65,25 @@ StateGeneratorProvider.prototype.request = function (domainObject, options) {
 
   var data = [];
   while (start <= end && data.length < 5000) {
-    data.push(pointForTimestamp(start, duration, domainObject.name));
+    const point = pointForTimestamp(start, duration, domainObject.name);
+
+    if (!this.shouldBeFiltered(point, options)) {
+      data.push(point);
+    }
     start += duration;
   }
 
   return Promise.resolve(data);
 };
+
+StateGeneratorProvider.prototype.shouldBeFiltered = function (point, options) {
+  const valueToFilter = options?.filters?.state?.equals?.[0];
+
+  if (!valueToFilter) {
+    return false;
+  }
+
+  const { value } = point;
+
+  return value !== Number(valueToFilter);
+};

src/api/telemetry/TelemetryAPI.js

@@ -250,6 +250,90 @@ export default class TelemetryAPI {
     return options;
   }
 
+  /**
+   * Sanitizes objects for consistent serialization by:
+   * 1. Removing non-plain objects (class instances) and functions
+   * 2. Sorting object keys alphabetically to ensure consistent ordering
+   * 3. Recursively processing nested objects
+   *
+   * Note: When used as a JSON.stringify replacer, this function will process objects
+   * twice - once for the initial sorting and again when JSON.stringify processes the
+   * sorted result. This is acceptable for small options objects, which is the
+   * intended use case.
+   */
+  sanitizeForSerialization(key, value) {
+    // Handle null and primitives directly
+    if (value === null || typeof value !== 'object') {
+      return value;
+    }
+
+    // Remove functions and non-plain objects by returning undefined
+    if (typeof value === 'function' || Object.getPrototypeOf(value) !== Object.prototype) {
+      return undefined;
+    }
+
+    // Handle plain objects
+    const sortedObject = {};
+    const keys = Object.keys(value).sort();
+    for (const objectKey of keys) {
+      const itemValue = value[objectKey];
+      const sanitizedValue = this.sanitizeForSerialization(objectKey, itemValue);
+      sortedObject[objectKey] = sanitizedValue;
+    }
+
+    return sortedObject;
+  }
+
+  /**
+   * Generates a numeric hash value for an options object. The hash is consistent
+   * for equivalent option objects regardless of property order.
+   *
+   * This is used to create compact, unique cache keys for telemetry subscriptions with
+   * different options configurations. The hash function ensures that identical options
+   * objects will always generate the same hash value, while different options objects
+   * (even with small differences) will generate different hash values.
+   *
+   * @private
+   * @param {Object} options The options object to hash
+   * @returns {number} A positive integer hash of the options object
+   */
+  #hashOptions(options) {
+    const sanitizedOptionsString = JSON.stringify(
+      options,
+      this.sanitizeForSerialization.bind(this)
+    );
+
+    let hash = 0;
+    const prime = 31;
+    const modulus = 1e9 + 9; // Large prime number
+
+    for (let i = 0; i < sanitizedOptionsString.length; i++) {
+      const char = sanitizedOptionsString.charCodeAt(i);
+      // Calculate new hash value while keeping numbers manageable
+      hash = Math.floor((hash * prime + char) % modulus);
+    }
+
+    return Math.abs(hash);
+  }
+
+  /**
+   * Generates a unique cache key for a telemetry subscription based on the
+   * domain object identifier and options (which includes strategy).
+   *
+   * Uses a hash of the options object to create compact cache keys while still
+   * ensuring unique keys for different subscription configurations.
+   *
+   * @private
+   * @param {import('openmct').DomainObject} domainObject The domain object being subscribed to
+   * @param {Object} options The subscription options object (including strategy)
+   * @returns {string} A unique key string for caching the subscription
+   */
+  #getSubscriptionCacheKey(domainObject, options) {
+    const keyString = makeKeyString(domainObject.identifier);
+
+    return `${keyString}:${this.#hashOptions(options)}`;
+  }
+
   /**
    * Register a request interceptor that transforms a request via module:openmct.TelemetryAPI.request
    * The request will be modified when it is received and will be returned in it's modified state
@@ -418,16 +502,14 @@ export default class TelemetryAPI {
       this.#subscribeCache = {};
     }
 
-    const keyString = makeKeyString(domainObject.identifier);
     const supportedStrategy = supportsBatching ? requestedStrategy : SUBSCRIBE_STRATEGY.LATEST;
     // Override the requested strategy with the strategy supported by the provider
     const optionsWithSupportedStrategy = {
       ...options,
       strategy: supportedStrategy
     };
-    // If batching is supported, we need to cache a subscription for each strategy -
-    // latest and batched.
-    const cacheKey = `${keyString}:${supportedStrategy}`;
+
+    const cacheKey = this.#getSubscriptionCacheKey(domainObject, optionsWithSupportedStrategy);
     let subscriber = this.#subscribeCache[cacheKey];
 
     if (!subscriber) {

src/plugins/filters/README.md

@@ -27,9 +27,9 @@ To define a filter, you'll need to add a new `filter` property to the domain obj
         singleSelectionThreshold: true,
         comparator: 'equals',
         possibleValues: [
-            { name: 'Apple', value: 'apple' },
-            { name: 'Banana', value: 'banana' },
-            { name: 'Orange', value: 'orange' }
+            { label: 'Apple', value: 'apple' },
+            { label: 'Banana', value: 'banana' },
+            { label: 'Orange', value: 'orange' }
         ]
     }]
 }