Skip to content

Commit

Permalink
fix(SharedState): do not propagate event parameters on first onUpdate…
Browse files Browse the repository at this point in the history
… call when executeListener is true
  • Loading branch information
b-ma committed Oct 1, 2024
1 parent 6970543 commit 7e372fa
Show file tree
Hide file tree
Showing 11 changed files with 167 additions and 154 deletions.
10 changes: 10 additions & 0 deletions src/common/SharedState.js
Original file line number Diff line number Diff line change
Expand Up @@ -731,6 +731,16 @@ ${JSON.stringify(initValues, null, 2)}`);

if (executeListener === true) {
const currentValues = this.getValues();
// filter `event: true` parameters from currentValues, this is missleading
// as we are in the context of a callback, not from an active read
const schema = this.getSchema();

for (let name in schema) {
if (schema[name].event === true) {
delete currentValues[name];
}
}

const oldValues = {};
const context = null;
listener(currentValues, oldValues, context);
Expand Down
122 changes: 71 additions & 51 deletions tests/states/SharedState.spec.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { delay, getTime } from '@ircam/sc-utils';
import { delay } from '@ircam/sc-utils';
import { assert } from 'chai';

import { Server } from '../../src/server/index.js';
Expand Down Expand Up @@ -125,7 +125,7 @@ describe('# SharedState', () => {
let asyncCallbackCalled = false;

a.onUpdate(async updates => {
await new Promise(resolve => setTimeout(resolve, 100));
await delay(10);
asyncCallbackCalled = true;
});

Expand Down Expand Up @@ -315,8 +315,7 @@ describe('# SharedState', () => {
unsubsribe();

await a.set({ int: 1 });

await new Promise(resolve => setTimeout(resolve, 100));
await delay(10);

assert.equal(onUpdateCalled, false);
await a.delete();
Expand All @@ -328,7 +327,7 @@ describe('# SharedState', () => {
let onUpdateCalled = false;
const unsubsribe = a.onUpdate(updates => { onUpdateCalled = true; }, false);

await new Promise(resolve => setTimeout(resolve, 100));
await delay(10);

assert.equal(onUpdateCalled, false);
await a.delete();
Expand All @@ -345,12 +344,33 @@ describe('# SharedState', () => {
assert.deepEqual(context, null);
}, true);

await new Promise(resolve => setTimeout(resolve, 100));
await delay(10);

assert.equal(onUpdateCalled, true);
await a.delete();
});

it('should not propagate event parameters on first call if `executeListener=true`', async () => {
server.stateManager.registerSchema('with-event', {
bool: { type: 'boolean', event: true, },
int: { type: 'integer', default: 20, },
});
const state = await server.stateManager.create('with-event');

let onUpdateCalled = false;
state.onUpdate((newValues, oldValues, context) => {
onUpdateCalled = true;
assert.deepEqual(newValues, { int: 20 });
assert.deepEqual(oldValues, {});
assert.deepEqual(context, null);
}, true);

await delay(10);

assert.equal(onUpdateCalled, true);
server.stateManager.deleteSchema('with-event');
});

it('should copy stored value for "any" type to have a predictable behavior', async () => {
server.stateManager.registerSchema('test-any', {
any: {
Expand Down Expand Up @@ -400,7 +420,7 @@ describe('# SharedState', () => {
assert.equal(a2.get('bool'), true); // this one is still attached

a0.delete();
await new Promise(resolve => setTimeout(resolve, 200));
await delay(10);

if (subscribeCalled) {
assert.fail('subscribe should not be called after detach');
Expand Down Expand Up @@ -613,6 +633,50 @@ describe('# SharedState - filtered attached state', () => {
});
});

describe(`## set(updates)`, () => {
it(`should throw early if trying to set modify a param which is not filtered`, async () => {
const filter = ['bool', 'string'];
const owned = await server.stateManager.create('filtered');
const attached = await client.stateManager.attach('filtered', filter);
let onUpdateCalled = false;
let errored = false;

owned.onUpdate(() => onUpdateCalled = true);

try {
await attached.set({ int: 42 });
} catch (err) {
console.log(err.message);
errored = true;
}

await delay(20);

assert.isTrue(errored);
assert.isFalse(onUpdateCalled);
});
});

describe(`## get(name)`, () => {
it(`should throw if trying to access a param which is not filtered`, async () => {
const filter = ['bool', 'string'];
const owned = await server.stateManager.create('filtered');
const attached = await client.stateManager.attach('filtered', filter);
let errored = false;

try {
await attached.get('int');
} catch (err) {
console.log(err.message);
errored = true;
}

await delay(20);

assert.isTrue(errored);
});
});

describe(`## onUpdate(callback)`, () => {
it(`should propagate only filtered keys`, async () => {
const filter = ['bool', 'string'];
Expand Down Expand Up @@ -660,50 +724,6 @@ describe('# SharedState - filtered attached state', () => {
});
});

describe(`## set(updates)`, () => {
it(`should throw early if trying to set modify a param which is not filtered`, async () => {
const filter = ['bool', 'string'];
const owned = await server.stateManager.create('filtered');
const attached = await client.stateManager.attach('filtered', filter);
let onUpdateCalled = false;
let errored = false;

owned.onUpdate(() => onUpdateCalled = true);

try {
await attached.set({ int: 42 });
} catch (err) {
console.log(err.message);
errored = true;
}

await delay(20);

assert.isTrue(errored);
assert.isFalse(onUpdateCalled);
});
});

describe(`## get(name)`, () => {
it(`should throw if trying to access a param which is not filtered`, async () => {
const filter = ['bool', 'string'];
const owned = await server.stateManager.create('filtered');
const attached = await client.stateManager.attach('filtered', filter);
let errored = false;

try {
await attached.get('int');
} catch (err) {
console.log(err.message);
errored = true;
}

await delay(20);

assert.isTrue(errored);
});
});

describe(`## getUnsafe(name)`, () => {
it(`should throw if trying to access a param which is not filtered`, async () => {
const filter = ['bool', 'string'];
Expand Down
6 changes: 3 additions & 3 deletions types/client/Client.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ declare class Client {
get status(): "idle" | "inited" | "started" | "errored";
/**
* The `init` method is part of the initialization lifecycle of the `soundworks`
* client. Most of the time, the `init` method will be implicitly called by the
* client. Most of the time, this method will be implicitly executed by the
* {@link Client#start} method.
*
* In some situations you might want to call this method manually, in such cases
Expand All @@ -140,8 +140,8 @@ declare class Client {
init(): Promise<void>;
/**
* The `start` method is part of the initialization lifecycle of the `soundworks`
* client. The `start` method will implicitly call the {@link Client#init}
* method if it has not been called manually.
* client. This method will implicitly execute {@link Client#init} method if it
* has not been called manually.
*
* What it does:
* - implicitly call {@link Client#init} if not done manually
Expand Down
24 changes: 0 additions & 24 deletions types/client/ClientPluginManager.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,30 +62,6 @@ declare class ClientPluginManager extends BasePluginManager {
* @param {Client} client - The soundworks client instance.
*/
constructor(client: Client);
/**
* Register a plugin into the manager.
*
* _A plugin must always be registered both on client-side and on server-side_
*
* Refer to the plugin documentation to check its options and proper way of
* registering it.
*
* @param {string} id - Unique id of the plugin. Enables the registration of the
* same plugin factory under different ids.
* @param {Function} factory - Factory function that returns the Plugin class.
* @param {object} [options={}] - Options to configure the plugin.
* @param {array} [deps=[]] - List of plugins' names the plugin depends on, i.e.
* the plugin initialization will start only after the plugins it depends on are
* fully started themselves.
* @see {@link ClientPluginManager#register}
* @see {@link ServerPluginManager#register}
* @example
* // client-side
* client.pluginManager.register('user-defined-id', pluginFactory);
* // server-side
* server.pluginManager.register('user-defined-id', pluginFactory);
*/
register(id: string, factory: Function, options?: object, deps?: any[]): void;
/**
* Retrieve an fully started instance of a registered plugin.
*
Expand Down
6 changes: 3 additions & 3 deletions types/common/BasePluginManager.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,10 @@ declare class BasePluginManager {
*
* @param {string} id - Unique id of the plugin. Enables the registration of the
* same plugin factory under different ids.
* @param {Function} factory - Factory function that returns the Plugin class.
* @param {Function} ctor - The class returned by the plugin factory method.
* @param {object} [options={}] - Options to configure the plugin.
* @param {array} [deps=[]] - List of plugins' names the plugin depends on, i.e.
* the plugin initialization will start only after the plugins it depends on are
* the plugin initialization will begin only after the plugins it depends on are
* fully started themselves.
* @see {@link ClientPluginManager#register}
* @see {@link ServerPluginManager#register}
Expand All @@ -72,7 +72,7 @@ declare class BasePluginManager {
* // server-side
* server.pluginManager.register('user-defined-id', pluginFactory);
*/
register(id: string, ctor: any, options?: object, deps?: any[]): void;
register(id: string, ctor: Function, options?: object, deps?: any[]): void;
/**
* Manually add a dependency to a given plugin.
*
Expand Down
28 changes: 15 additions & 13 deletions types/common/ParameterBag.d.ts
Original file line number Diff line number Diff line change
@@ -1,36 +1,38 @@
export namespace sharedOptions {
let nullable: boolean;
let event: boolean;
let required: boolean;
let metas: {};
let filterChange: boolean;
let immediate: boolean;
}
export namespace types {
export namespace boolean {
let required: string[];
const defaultOptions: any;
function coerceFunction(name: any, def: any, value: any): boolean;
}
export namespace string {
let required_1: string[];
export { required_1 as required };
export const defaultOptions: any;
export function coerceFunction(name: any, def: any, value: any): boolean;
}
export namespace string {
let required_2: string[];
export { required_2 as required };
const defaultOptions_1: any;
export { defaultOptions_1 as defaultOptions };
export function coerceFunction_1(name: any, def: any, value: any): string;
export { coerceFunction_1 as coerceFunction };
}
export namespace integer {
let required_2: string[];
export { required_2 as required };
let required_3: string[];
export { required_3 as required };
const defaultOptions_2: any;
export { defaultOptions_2 as defaultOptions };
export function sanitizeSchema(def: any): any;
export function coerceFunction_2(name: any, def: any, value: any): number;
export { coerceFunction_2 as coerceFunction };
}
export namespace float {
let required_3: string[];
export { required_3 as required };
let required_4: string[];
export { required_4 as required };
const defaultOptions_3: any;
export { defaultOptions_3 as defaultOptions };
export function sanitizeSchema_1(def: any): any;
Expand All @@ -39,17 +41,17 @@ export namespace types {
export { coerceFunction_3 as coerceFunction };
}
export namespace _enum {
let required_4: string[];
export { required_4 as required };
let required_5: string[];
export { required_5 as required };
const defaultOptions_4: any;
export { defaultOptions_4 as defaultOptions };
export function coerceFunction_4(name: any, def: any, value: any): any;
export { coerceFunction_4 as coerceFunction };
}
export { _enum as enum };
export namespace any {
let required_5: string[];
export { required_5 as required };
let required_6: string[];
export { required_6 as required };
const defaultOptions_5: any;
export { defaultOptions_5 as defaultOptions };
export function coerceFunction_5(name: any, def: any, value: any): any;
Expand Down
Loading

0 comments on commit 7e372fa

Please sign in to comment.