Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 14 commits into from
Dec 4, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 117 additions & 0 deletions e2e/tests/functional/plugins/displayLayout/displayLayout.e2e.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -507,8 +507,125 @@ 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
// Wait for some data to populate
// eslint-disable-next-line playwright/no-wait-for-timeout
await page.waitForTimeout(2000); // Wait for a few state changes
jvigliotta marked this conversation as resolved.
Show resolved Hide resolved

// 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);
Expand Down
10 changes: 10 additions & 0 deletions example/generator/GeneratorMetadataProvider.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
28 changes: 23 additions & 5 deletions example/generator/StateGeneratorProvider.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand All @@ -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);
};
121 changes: 117 additions & 4 deletions src/api/telemetry/TelemetryAPI.js
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,121 @@ export default class TelemetryAPI {
return options;
}

/**
* Canonicalizes options object by creating a standardized version with sorted keys
* and recursively processed values. This ensures consistent object representation
* regardless of property order.
*
* Used internally to generate consistent hashes for caching subscription options.
*
* @private
* @param {Object|Array|*} options The options object to canonicalize
* @returns {Object|Array|*} A canonicalized version of the input where object keys are sorted
*/
#canonicalizeOptions(options) {
jvigliotta marked this conversation as resolved.
Show resolved Hide resolved
if (typeof options !== 'object' || options === null) {
return options;
}

if (Array.isArray(options)) {
return options.map((item) => this.#canonicalizeOptions(item));
}

return Object.keys(options)
.sort()
.reduce((acc, key) => {
acc[key] = this.#canonicalizeOptions(options[key]);
return acc;
}, {});
}

/**
* Filters out class instances and functions from an object, returning a plain
* data object suitable for JSON serialization.
*
* @private
* @param {Object|Array|*} value The value to sanitize
* @returns {Object|Array|*} A sanitized copy with class instances and functions removed
*/
#sanitizeForSerialization(value) {
if (value === null || value === undefined) {
return value;
}

// Handle primitive types, arrays and objects
if (typeof value !== 'object') {
return typeof value !== 'function' ? value : undefined;
}

if (Array.isArray(value)) {
return value.map((item) => this.#sanitizeForSerialization(item));
}

if (Object.getPrototypeOf(value) === Object.prototype) {
const result = {};

for (const key of Object.keys(value)) {
const sanitizedValue = this.#sanitizeForSerialization(value[key]);

if (sanitizedValue !== undefined) {
result[key] = sanitizedValue;
}
}
return result;
}

return undefined;
}

/**
* 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 sanitizedOptions = this.#sanitizeForSerialization(options);
jvigliotta marked this conversation as resolved.
Show resolved Hide resolved
const canonicalOptions = this.#canonicalizeOptions(sanitizedOptions);
const str = JSON.stringify(canonicalOptions);

let hash = 0;
const prime = 31;
const modulus = 1e9 + 9; // Large prime number

for (let i = 0; i < str.length; i++) {
const char = str.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
Expand Down Expand Up @@ -418,16 +533,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) {
Expand Down
6 changes: 3 additions & 3 deletions src/plugins/filters/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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' }
]
}]
}
Expand Down
Loading