mirror of
https://github.com/nasa/openmct.git
synced 2024-12-18 12:56:25 +00:00
[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
This commit is contained in:
parent
ba4d8a428b
commit
61b982ab99
@ -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);
|
||||
|
@ -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
|
||||
}
|
||||
|
@ -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);
|
||||
};
|
||||
|
@ -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) {
|
||||
|
@ -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' }
|
||||
]
|
||||
}]
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user