Skip to content

Commit

Permalink
feat: unique listener names for lifecycle events
Browse files Browse the repository at this point in the history
  • Loading branch information
mshanemc committed Sep 25, 2023
1 parent 209aac0 commit 84ec4ab
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 10 deletions.
30 changes: 24 additions & 6 deletions src/lifecycleEvents.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@ export class Lifecycle {
public static readonly warningEventName = 'warning';
private logger?: Logger;

private constructor(private readonly listeners: Dictionary<callback[]> = {}) {}
private constructor(
private readonly listeners: Dictionary<callback[]> = {},
private readonly uniqueListeners: Map<string, Map<string, callback>> = new Map<string, Map<string, callback>>()
) {}

/**
* return the package.json version of the sfdx-core library.
Expand Down Expand Up @@ -87,7 +90,7 @@ export class Lifecycle {
const oldInstance = global.salesforceCoreLifecycle;
// use the newer version and transfer any listeners from the old version
// object spread keeps them from being references
global.salesforceCoreLifecycle = new Lifecycle({ ...oldInstance.listeners });
global.salesforceCoreLifecycle = new Lifecycle({ ...oldInstance.listeners }, oldInstance.uniqueListeners);
// clean up any listeners on the old version
Object.keys(oldInstance.listeners).map((eventName) => {
oldInstance.removeAllListeners(eventName);
Expand All @@ -111,6 +114,7 @@ export class Lifecycle {
*/
public removeAllListeners(eventName: string): void {
this.listeners[eventName] = [];
this.uniqueListeners.delete(eventName);
}

/**
Expand All @@ -119,7 +123,9 @@ export class Lifecycle {
* @param eventName The name of the event to get listeners of
*/
public getListeners(eventName: string): callback[] {
const listeners = this.listeners[eventName];
const listeners = this.listeners[eventName]?.concat(
Array.from((this.uniqueListeners.get(eventName) ?? []).values()) ?? []
);
if (listeners) {
return listeners;
} else {
Expand Down Expand Up @@ -150,8 +156,9 @@ export class Lifecycle {
*
* @param eventName The name of the event that is being listened for
* @param cb The callback function to run when the event is emitted
* @param uniqueListenerIdentifier A unique identifier for the listener. If a listener with the same identifier is already registered, a new one will not be added
*/
public on<T = AnyJson>(eventName: string, cb: (data: T) => Promise<void>): void {
public on<T = AnyJson>(eventName: string, cb: (data: T) => Promise<void>, uniqueListenerIdentifier?: string): void {
const listeners = this.getListeners(eventName);
if (listeners.length !== 0) {
if (!this.logger) {
Expand All @@ -165,8 +172,19 @@ export class Lifecycle {
} listeners will fire.`
);
}
listeners.push(cb);
this.listeners[eventName] = listeners;

if (uniqueListenerIdentifier) {
if (!this.uniqueListeners.has(eventName)) {
// nobody is listening to the event yet
this.uniqueListeners.set(eventName, new Map<string, callback>([[uniqueListenerIdentifier, cb]]));
} else if (!this.uniqueListeners.get(eventName)?.has(uniqueListenerIdentifier)) {
// the unique listener identifier is not already registered
this.uniqueListeners.get(eventName)?.set(uniqueListenerIdentifier, cb);
}
} else {
listeners.push(cb);
this.listeners[eventName] = listeners;
}
}

/**
Expand Down
88 changes: 84 additions & 4 deletions test/unit/lifecycleEventsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,45 @@ describe('lifecycleEvents', () => {
Lifecycle.getInstance().removeAllListeners('telemetry');
});

it("uniqueListeners adds listeners only when they don't already exist", async () => {
Lifecycle.getInstance().on(
'test1',
async (result) => {
// @ts-expect-error: called is a sinon spy property
fake.bar('test1', result);
},
'id000'
);
chai.expect(Lifecycle.getInstance().getListeners('test1').length).to.equal(1);
Lifecycle.getInstance().on(
'test1',
async (result) => {
// @ts-expect-error: called is a sinon spy property
fake.bar('test1', result);
},
'id001'
);
// both of these should be ignored
chai.expect(Lifecycle.getInstance().getListeners('test1').length).to.equal(2);
Lifecycle.getInstance().on(
'test1',
async (result) => {
// @ts-expect-error: called is a sinon spy property
fake.bar('test1', result);
},
'id000'
);
chai.expect(Lifecycle.getInstance().getListeners('test1').length).to.equal(2);
Lifecycle.getInstance().on(
'test1',
async (result) => {
// @ts-expect-error: called is a sinon spy property
fake.bar('test1', result);
},
'id001'
);
Lifecycle.getInstance().removeAllListeners('test1');
});
it('successful event registration and emitting causes the callback to be called', async () => {
Lifecycle.getInstance().on('test1', async (result) => {
// @ts-expect-error: called is a sinon spy property
Expand Down Expand Up @@ -131,13 +170,24 @@ describe('lifecycleEvents', () => {
// @ts-expect-error: called is a sinon spy property
fake.bar('test5', result);
});
Lifecycle.getInstance().on(
'test5',
async (result) => {
// @ts-expect-error: called is a sinon spy property
fake.bar('test5', result);
},
'id000'
);
await Lifecycle.getInstance().emit('test5', 'Success');
chai.expect(fakeSpy.callCount).to.be.equal(1);
chai.expect(fakeSpy.callCount).to.be.equal(2);
chai.expect(fakeSpy.args[0][1]).to.be.equal('Success');

chai.expect(fakeSpy.args[1][1]).to.be.equal('Success');
loggerSpy.resetHistory();
fakeSpy.resetHistory();
Lifecycle.getInstance().removeAllListeners('test5');
await Lifecycle.getInstance().emit('test5', 'Failure: Listener Removed');
chai.expect(fakeSpy.callCount).to.be.equal(1);
chai.expect(fakeSpy.callCount).to.be.equal(0);

chai.expect(loggerSpy.callCount).to.be.equal(1);
chai
.expect(loggerSpy.args[0][0])
Expand All @@ -146,7 +196,7 @@ describe('lifecycleEvents', () => {
);
});

it('getListeners works', async () => {
it('getListeners works, including uniqueListeners', async () => {
const x = async (result: Record<string, unknown>) => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
fake.bar('test6', result);
Expand All @@ -159,6 +209,36 @@ describe('lifecycleEvents', () => {
Lifecycle.getInstance().removeAllListeners('test6');
});

it('getListeners works (unique Listeners)', async () => {
const x = async (result: Record<string, unknown>) => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
fake.bar('test6', result);
};
Lifecycle.getInstance().on('test6', x, 'id000');

chai.expect(Lifecycle.getInstance().getListeners('test6')).to.have.lengthOf(1);
chai.expect(Lifecycle.getInstance().getListeners('test6')[0]).to.deep.equal(x);

chai.expect(Lifecycle.getInstance().getListeners('undefinedKey').length).to.be.equal(0);
Lifecycle.getInstance().removeAllListeners('test6');
});

it('getListeners works (mixed unique and non-unique)', async () => {
const x = async (result: Record<string, unknown>) => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
fake.bar('test6', result);
};
Lifecycle.getInstance().on('test6', x);
Lifecycle.getInstance().on('test6', x, 'id000');

chai.expect(Lifecycle.getInstance().getListeners('test6')).to.have.lengthOf(2);
chai.expect(Lifecycle.getInstance().getListeners('test6')[0]).to.deep.equal(x);
chai.expect(Lifecycle.getInstance().getListeners('test6')[1]).to.deep.equal(x);

chai.expect(Lifecycle.getInstance().getListeners('undefinedKey').length).to.be.equal(0);
Lifecycle.getInstance().removeAllListeners('test6');
});

it('will use a newer version and transfer the listeners', () => {
// the original
const lifecycle = Lifecycle.getInstance();
Expand Down

3 comments on commit 84ec4ab

@svc-cli-bot
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logger Benchmarks - ubuntu-latest

Benchmark suite Current: 84ec4ab Previous: 9c419e7 Ratio
Child logger creation 542867 ops/sec (±0.53%) 542112 ops/sec (±3.73%) 1.00
Logging a string on root logger 597916 ops/sec (±10.58%) 577947 ops/sec (±17.68%) 0.97
Logging an object on root logger 407304 ops/sec (±19.32%) 423355 ops/sec (±13.28%) 1.04
Logging an object with a message on root logger 294538 ops/sec (±14.40%) 274006 ops/sec (±17.65%) 0.93
Logging an object with a redacted prop on root logger 11700 ops/sec (±188.02%) 11447 ops/sec (±188.19%) 0.98
Logging a nested 3-level object on root logger 265723 ops/sec (±12.20%) 259153 ops/sec (±15.89%) 0.98

This comment was automatically generated by workflow using github-action-benchmark.

@svc-cli-bot
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logger Benchmarks - windows-latest

Benchmark suite Current: 84ec4ab Previous: 9c419e7 Ratio
Child logger creation 455013 ops/sec (±8.95%) 485997 ops/sec (±5.89%) 1.07
Logging a string on root logger 568568 ops/sec (±13.23%) 594561 ops/sec (±10.50%) 1.05
Logging an object on root logger 206582 ops/sec (±94.05%) 320118 ops/sec (±16.55%) 1.55
Logging an object with a message on root logger 270143 ops/sec (±17.23%) 148794 ops/sec (±18.60%) 0.55
Logging an object with a redacted prop on root logger 10839 ops/sec (±189.18%) 163900 ops/sec (±21.23%) 15.12
Logging a nested 3-level object on root logger 260027 ops/sec (±11.19%) 132265 ops/sec (±21.15%) 0.51

This comment was automatically generated by workflow using github-action-benchmark.

@svc-cli-bot
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Logger Benchmarks - windows-latest'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 84ec4ab Previous: 9c419e7 Ratio
Logging an object with a redacted prop on root logger 10839 ops/sec (±189.18%) 163900 ops/sec (±21.23%) 15.12

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.