Skip to content

Commit 93b5853

Browse files
raju-optiraju-opti
committed
refactor notification center
1 parent 87f6e67 commit 93b5853

File tree

4 files changed

+47
-175
lines changed

4 files changed

+47
-175
lines changed

‎lib/notification_center/index.tests.js

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -63,47 +63,6 @@ describe('lib/core/notification_center', function() {
6363
});
6464

6565
context('the listener type is a valid type', function() {
66-
it('should return -1 if that same callback is already added', function() {
67-
var activateCallback;
68-
var decisionCallback;
69-
var logEventCallback;
70-
var configUpdateCallback;
71-
var trackCallback;
72-
// add a listener for each type
73-
notificationCenterInstance.addNotificationListener(NOTIFICATION_TYPES.ACTIVATE, activateCallback);
74-
notificationCenterInstance.addNotificationListener(NOTIFICATION_TYPES.DECISION, decisionCallback);
75-
notificationCenterInstance.addNotificationListener(NOTIFICATION_TYPES.LOG_EVENT, logEventCallback);
76-
notificationCenterInstance.addNotificationListener(
77-
NOTIFICATION_TYPES.OPTIMIZELY_CONFIG_UPDATE,
78-
configUpdateCallback
79-
);
80-
notificationCenterInstance.addNotificationListener(NOTIFICATION_TYPES.TRACK, trackCallback);
81-
// assertions
82-
assert.strictEqual(
83-
notificationCenterInstance.addNotificationListener(NOTIFICATION_TYPES.ACTIVATE, activateCallback),
84-
-1
85-
);
86-
assert.strictEqual(
87-
notificationCenterInstance.addNotificationListener(NOTIFICATION_TYPES.DECISION, decisionCallback),
88-
-1
89-
);
90-
assert.strictEqual(
91-
notificationCenterInstance.addNotificationListener(NOTIFICATION_TYPES.LOG_EVENT, logEventCallback),
92-
-1
93-
);
94-
assert.strictEqual(
95-
notificationCenterInstance.addNotificationListener(
96-
NOTIFICATION_TYPES.OPTIMIZELY_CONFIG_UPDATE,
97-
configUpdateCallback
98-
),
99-
-1
100-
);
101-
assert.strictEqual(
102-
notificationCenterInstance.addNotificationListener(NOTIFICATION_TYPES.TRACK, trackCallback),
103-
-1
104-
);
105-
});
106-
10766
it('should return an id (listenerId) > 0 of the notification listener if callback is not already added', function() {
10867
var activateCallback;
10968
var decisionCallback;

‎lib/notification_center/index.ts

Lines changed: 43 additions & 127 deletions
Original file line numberDiff line numberDiff line change
@@ -23,25 +23,15 @@ import {
2323

2424
import { NOTIFICATION_TYPES } from './type';
2525
import { NotificationType, NotificationPayload } from './type';
26-
import { Consumer } from '../utils/type';
26+
import { Consumer, Fn } from '../utils/type';
27+
import { EventEmitter } from '../utils/event_emitter/event_emitter';
2728

2829
const MODULE_NAME = 'NOTIFICATION_CENTER';
2930

3031
interface NotificationCenterOptions {
3132
logger: LogHandler;
3233
errorHandler: ErrorHandler;
3334
}
34-
35-
interface ListenerEntry {
36-
id: number;
37-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
38-
callback: (notificationData: any) => void;
39-
}
40-
41-
type NotificationListeners = {
42-
[key: string]: ListenerEntry[];
43-
}
44-
4535
export interface NotificationCenter {
4636
addNotificationListener<N extends NotificationType>(
4737
notificationType: N,
@@ -68,8 +58,11 @@ export interface NotificationSender {
6858
export class DefaultNotificationCenter implements NotificationCenter, NotificationSender {
6959
private logger: LogHandler;
7060
private errorHandler: ErrorHandler;
71-
private notificationListeners: NotificationListeners;
72-
private listenerId: number;
61+
private callbacks: Map<Fn, boolean> = new Map();
62+
63+
private removerId = 1;
64+
private eventEmitter: EventEmitter<NotificationPayload> = new EventEmitter();
65+
private removers: Map<number, Fn> = new Map();
7366

7467
/**
7568
* @constructor
@@ -80,13 +73,6 @@ export class DefaultNotificationCenter implements NotificationCenter, Notificati
8073
constructor(options: NotificationCenterOptions) {
8174
this.logger = options.logger;
8275
this.errorHandler = options.errorHandler;
83-
this.notificationListeners = {};
84-
objectValues(NOTIFICATION_TYPES).forEach(
85-
(notificationTypeEnum) => {
86-
this.notificationListeners[notificationTypeEnum] = [];
87-
}
88-
);
89-
this.listenerId = 1;
9076
}
9177

9278
/**
@@ -103,43 +89,36 @@ export class DefaultNotificationCenter implements NotificationCenter, Notificati
10389
notificationType: N,
10490
callback: Consumer<NotificationPayload[N]>
10591
): number {
106-
try {
107-
const notificationTypeValues: string[] = objectValues(NOTIFICATION_TYPES);
108-
const isNotificationTypeValid = notificationTypeValues.indexOf(notificationType) > -1;
109-
if (!isNotificationTypeValid) {
110-
return -1;
111-
}
112-
113-
if (!this.notificationListeners[notificationType]) {
114-
this.notificationListeners[notificationType] = [];
115-
}
116-
117-
let callbackAlreadyAdded = false;
118-
(this.notificationListeners[notificationType] || []).forEach(
119-
(listenerEntry) => {
120-
if (listenerEntry.callback === callback) {
121-
callbackAlreadyAdded = true;
122-
return;
123-
}
124-
});
125-
126-
if (callbackAlreadyAdded) {
127-
return -1;
128-
}
129-
130-
this.notificationListeners[notificationType].push({
131-
id: this.listenerId,
132-
callback: callback,
133-
});
134-
135-
const returnId = this.listenerId;
136-
this.listenerId += 1;
137-
return returnId;
138-
} catch (e: any) {
139-
this.logger.log(LOG_LEVEL.ERROR, e.message);
140-
this.errorHandler.handleError(e);
92+
const notificationTypeValues: string[] = objectValues(NOTIFICATION_TYPES);
93+
const isNotificationTypeValid = notificationTypeValues.indexOf(notificationType) > -1;
94+
if (!isNotificationTypeValid) {
14195
return -1;
14296
}
97+
98+
const returnId = this.removerId++;
99+
const remover = this.eventEmitter.on(
100+
notificationType, this.wrapWithErrorHandling(notificationType, callback));
101+
this.removers.set(returnId, remover);
102+
return returnId;
103+
}
104+
105+
private wrapWithErrorHandling<N extends NotificationType>(
106+
notificationType: N,
107+
callback: Consumer<NotificationPayload[N]>
108+
): Consumer<NotificationPayload[N]> {
109+
return (notificationData: NotificationPayload[N]) => {
110+
try {
111+
callback(notificationData);
112+
} catch (ex: any) {
113+
this.logger.log(
114+
LOG_LEVEL.ERROR,
115+
LOG_MESSAGES.NOTIFICATION_LISTENER_EXCEPTION,
116+
MODULE_NAME,
117+
notificationType,
118+
ex.message,
119+
);
120+
}
121+
};
143122
}
144123

145124
/**
@@ -149,70 +128,27 @@ export class DefaultNotificationCenter implements NotificationCenter, Notificati
149128
* otherwise.
150129
*/
151130
removeNotificationListener(listenerId: number): boolean {
152-
try {
153-
let indexToRemove: number | undefined;
154-
let typeToRemove: string | undefined;
155-
156-
Object.keys(this.notificationListeners).some(
157-
(notificationType) => {
158-
const listenersForType = this.notificationListeners[notificationType];
159-
(listenersForType || []).every((listenerEntry, i) => {
160-
if (listenerEntry.id === listenerId) {
161-
indexToRemove = i;
162-
typeToRemove = notificationType;
163-
return false;
164-
}
165-
166-
return true;
167-
});
168-
169-
if (indexToRemove !== undefined && typeToRemove !== undefined) {
170-
return true;
171-
}
172-
173-
return false;
174-
}
175-
);
176-
177-
if (indexToRemove !== undefined && typeToRemove !== undefined) {
178-
this.notificationListeners[typeToRemove].splice(indexToRemove, 1);
179-
return true;
180-
}
181-
} catch (e: any) {
182-
this.logger.log(LOG_LEVEL.ERROR, e.message);
183-
this.errorHandler.handleError(e);
131+
const remover = this.removers.get(listenerId);
132+
if (remover) {
133+
remover();
134+
return true;
184135
}
185-
186-
return false;
136+
return false
187137
}
188138

189139
/**
190140
* Removes all previously added notification listeners, for all notification types
191141
*/
192142
clearAllNotificationListeners(): void {
193-
try {
194-
objectValues(NOTIFICATION_TYPES).forEach(
195-
(notificationTypeEnum) => {
196-
this.notificationListeners[notificationTypeEnum] = [];
197-
}
198-
);
199-
} catch (e: any) {
200-
this.logger.log(LOG_LEVEL.ERROR, e.message);
201-
this.errorHandler.handleError(e);
202-
}
143+
this.eventEmitter.removeAllListeners();
203144
}
204145

205146
/**
206147
* Remove all previously added notification listeners for the argument type
207148
* @param {NOTIFICATION_TYPES} notificationType One of NOTIFICATION_TYPES
208149
*/
209150
clearNotificationListeners(notificationType: NotificationType): void {
210-
try {
211-
this.notificationListeners[notificationType] = [];
212-
} catch (e: any) {
213-
this.logger.log(LOG_LEVEL.ERROR, e.message);
214-
this.errorHandler.handleError(e);
215-
}
151+
this.eventEmitter.removeListeners(notificationType);
216152
}
217153

218154
/**
@@ -225,27 +161,7 @@ export class DefaultNotificationCenter implements NotificationCenter, Notificati
225161
notificationType: N,
226162
notificationData: NotificationPayload[N]
227163
): void {
228-
try {
229-
(this.notificationListeners[notificationType] || []).forEach(
230-
(listenerEntry) => {
231-
const callback = listenerEntry.callback;
232-
try {
233-
callback(notificationData);
234-
} catch (ex: any) {
235-
this.logger.log(
236-
LOG_LEVEL.ERROR,
237-
LOG_MESSAGES.NOTIFICATION_LISTENER_EXCEPTION,
238-
MODULE_NAME,
239-
notificationType,
240-
ex.message,
241-
);
242-
}
243-
}
244-
);
245-
} catch (e: any) {
246-
this.logger.log(LOG_LEVEL.ERROR, e.message);
247-
this.errorHandler.handleError(e);
248-
}
164+
this.eventEmitter.emit(notificationType, notificationData);
249165
}
250166
}
251167

‎lib/optimizely/index.tests.js

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2385,13 +2385,6 @@ describe('lib/optimizely', function() {
23852385
sinon.assert.calledOnce(trackListener);
23862386
});
23872387

2388-
it('should only call the listener once after the same listener was added twice', function() {
2389-
optlyInstance.notificationCenter.addNotificationListener(NOTIFICATION_TYPES.ACTIVATE, activateListener);
2390-
optlyInstance.notificationCenter.addNotificationListener(NOTIFICATION_TYPES.ACTIVATE, activateListener);
2391-
optlyInstance.activate('testExperiment', 'testUser');
2392-
sinon.assert.calledOnce(activateListener);
2393-
});
2394-
23952388
it('should not add a listener with an invalid type argument', function() {
23962389
var listenerId = optlyInstance.notificationCenter.addNotificationListener(
23972390
'not a notification type',

‎lib/utils/event_emitter/event_emitter.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ export class EventEmitter<T> {
4747
}
4848
}
4949

50+
removeListeners<E extends keyof T>(eventName: E): void {
51+
this.listeners[eventName]?.clear();
52+
}
53+
5054
removeAllListeners(): void {
5155
this.listeners = {};
5256
}

0 commit comments

Comments
 (0)