Skip to content

Commit a1dc85e

Browse files
authored
Merge pull request #3223 from jerch/simplify_handler_interface
cleanup parser handler interface
2 parents 74ea558 + 3f659a1 commit a1dc85e

File tree

9 files changed

+600
-463
lines changed

9 files changed

+600
-463
lines changed

src/common/InputHandler.ts

Lines changed: 245 additions & 173 deletions
Large diffs are not rendered by default.

src/common/parser/DcsParser.test.ts

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,12 @@ class TestHandler implements IDcsHandler {
6161
public put(data: Uint32Array, start: number, end: number): void {
6262
this.output.push([this.msg, 'PUT', utf32ToString(data, start, end)]);
6363
}
64-
public unhook(success: boolean): void | boolean {
64+
public unhook(success: boolean): boolean {
6565
this.output.push([this.msg, 'UNHOOK', success]);
6666
if (this.returnFalse) {
6767
return false;
6868
}
69+
return true;
6970
}
7071
}
7172

@@ -84,7 +85,7 @@ describe('DcsParser', () => {
8485
});
8586
describe('handler registration', () => {
8687
it('setDcsHandler', () => {
87-
parser.setHandler(identifier({intermediates: '+', final: 'p'}), new TestHandler(reports, 'th'));
88+
parser.registerHandler(identifier({intermediates: '+', final: 'p'}), new TestHandler(reports, 'th'));
8889
parser.hook(identifier({intermediates: '+', final: 'p'}), Params.fromArray([1, 2, 3]));
8990
let data = toUtf32('Here comes');
9091
parser.put(data, 0, data.length);
@@ -100,7 +101,7 @@ describe('DcsParser', () => {
100101
]);
101102
});
102103
it('clearDcsHandler', () => {
103-
parser.setHandler(identifier({intermediates: '+', final: 'p'}), new TestHandler(reports, 'th'));
104+
parser.registerHandler(identifier({intermediates: '+', final: 'p'}), new TestHandler(reports, 'th'));
104105
parser.clearHandler(identifier({intermediates: '+', final: 'p'}));
105106
parser.hook(identifier({intermediates: '+', final: 'p'}), Params.fromArray([1, 2, 3]));
106107
let data = toUtf32('Here comes');
@@ -117,8 +118,8 @@ describe('DcsParser', () => {
117118
]);
118119
});
119120
it('addDcsHandler', () => {
120-
parser.setHandler(identifier({intermediates: '+', final: 'p'}), new TestHandler(reports, 'th1'));
121-
parser.addHandler(identifier({intermediates: '+', final: 'p'}), new TestHandler(reports, 'th2'));
121+
parser.registerHandler(identifier({intermediates: '+', final: 'p'}), new TestHandler(reports, 'th1'));
122+
parser.registerHandler(identifier({intermediates: '+', final: 'p'}), new TestHandler(reports, 'th2'));
122123
parser.hook(identifier({intermediates: '+', final: 'p'}), Params.fromArray([1, 2, 3]));
123124
let data = toUtf32('Here comes');
124125
parser.put(data, 0, data.length);
@@ -137,8 +138,8 @@ describe('DcsParser', () => {
137138
]);
138139
});
139140
it('addDcsHandler with return false', () => {
140-
parser.setHandler(identifier({intermediates: '+', final: 'p'}), new TestHandler(reports, 'th1'));
141-
parser.addHandler(identifier({intermediates: '+', final: 'p'}), new TestHandler(reports, 'th2', true));
141+
parser.registerHandler(identifier({intermediates: '+', final: 'p'}), new TestHandler(reports, 'th1'));
142+
parser.registerHandler(identifier({intermediates: '+', final: 'p'}), new TestHandler(reports, 'th2', true));
142143
parser.hook(identifier({intermediates: '+', final: 'p'}), Params.fromArray([1, 2, 3]));
143144
let data = toUtf32('Here comes');
144145
parser.put(data, 0, data.length);
@@ -157,8 +158,8 @@ describe('DcsParser', () => {
157158
]);
158159
});
159160
it('dispose handlers', () => {
160-
parser.setHandler(identifier({intermediates: '+', final: 'p'}), new TestHandler(reports, 'th1'));
161-
const dispo = parser.addHandler(identifier({intermediates: '+', final: 'p'}), new TestHandler(reports, 'th2', true));
161+
parser.registerHandler(identifier({intermediates: '+', final: 'p'}), new TestHandler(reports, 'th1'));
162+
const dispo = parser.registerHandler(identifier({intermediates: '+', final: 'p'}), new TestHandler(reports, 'th2', true));
162163
dispo.dispose();
163164
parser.hook(identifier({intermediates: '+', final: 'p'}), Params.fromArray([1, 2, 3]));
164165
let data = toUtf32('Here comes');
@@ -176,7 +177,7 @@ describe('DcsParser', () => {
176177
});
177178
describe('DcsHandlerFactory', () => {
178179
it('should be called once on end(true)', () => {
179-
parser.setHandler(identifier({intermediates: '+', final: 'p'}), new DcsHandler((data, params) => reports.push([params.toArray(), data])));
180+
parser.registerHandler(identifier({intermediates: '+', final: 'p'}), new DcsHandler((data, params) => { reports.push([params.toArray(), data]); return true; }));
180181
parser.hook(identifier({intermediates: '+', final: 'p'}), Params.fromArray([1, 2, 3]));
181182
let data = toUtf32('Here comes');
182183
parser.put(data, 0, data.length);
@@ -186,7 +187,7 @@ describe('DcsParser', () => {
186187
assert.deepEqual(reports, [[[1, 2, 3], 'Here comes the mouse!']]);
187188
});
188189
it('should not be called on end(false)', () => {
189-
parser.setHandler(identifier({intermediates: '+', final: 'p'}), new DcsHandler((data, params) => reports.push([params.toArray(), data])));
190+
parser.registerHandler(identifier({intermediates: '+', final: 'p'}), new DcsHandler((data, params) => { reports.push([params.toArray(), data]); return true; }));
190191
parser.hook(identifier({intermediates: '+', final: 'p'}), Params.fromArray([1, 2, 3]));
191192
let data = toUtf32('Here comes');
192193
parser.put(data, 0, data.length);
@@ -196,8 +197,8 @@ describe('DcsParser', () => {
196197
assert.deepEqual(reports, []);
197198
});
198199
it('should be disposable', () => {
199-
parser.setHandler(identifier({intermediates: '+', final: 'p'}), new DcsHandler((data, params) => reports.push(['one', params.toArray(), data])));
200-
const dispo = parser.addHandler(identifier({intermediates: '+', final: 'p'}), new DcsHandler((data, params) => reports.push(['two', params.toArray(), data])));
200+
parser.registerHandler(identifier({intermediates: '+', final: 'p'}), new DcsHandler((data, params) => { reports.push(['one', params.toArray(), data]); return true; }));
201+
const dispo = parser.registerHandler(identifier({intermediates: '+', final: 'p'}), new DcsHandler((data, params) => { reports.push(['two', params.toArray(), data]); return true; }));
201202
parser.hook(identifier({intermediates: '+', final: 'p'}), Params.fromArray([1, 2, 3]));
202203
let data = toUtf32('Here comes');
203204
parser.put(data, 0, data.length);
@@ -215,8 +216,8 @@ describe('DcsParser', () => {
215216
assert.deepEqual(reports, [['two', [1, 2, 3], 'Here comes the mouse!'], ['one', [1, 2, 3], 'some other data']]);
216217
});
217218
it('should respect return false', () => {
218-
parser.setHandler(identifier({intermediates: '+', final: 'p'}), new DcsHandler((data, params) => reports.push(['one', params.toArray(), data])));
219-
parser.addHandler(identifier({intermediates: '+', final: 'p'}), new DcsHandler((data, params) => { reports.push(['two', params.toArray(), data]); return false; }));
219+
parser.registerHandler(identifier({intermediates: '+', final: 'p'}), new DcsHandler((data, params) => { reports.push(['one', params.toArray(), data]); return true; }));
220+
parser.registerHandler(identifier({intermediates: '+', final: 'p'}), new DcsHandler((data, params) => { reports.push(['two', params.toArray(), data]); return false; }));
220221
parser.hook(identifier({intermediates: '+', final: 'p'}), Params.fromArray([1, 2, 3]));
221222
let data = toUtf32('Here comes');
222223
parser.put(data, 0, data.length);
@@ -227,7 +228,7 @@ describe('DcsParser', () => {
227228
});
228229
it('should work up to payload limit', function(): void {
229230
this.timeout(10000);
230-
parser.setHandler(identifier({intermediates: '+', final: 'p'}), new DcsHandler((data, params) => reports.push([params.toArray(), data])));
231+
parser.registerHandler(identifier({intermediates: '+', final: 'p'}), new DcsHandler((data, params) => { reports.push([params.toArray(), data]); return true; }));
231232
parser.hook(identifier({intermediates: '+', final: 'p'}), Params.fromArray([1, 2, 3]));
232233
const data = toUtf32('A'.repeat(1000));
233234
for (let i = 0; i < PAYLOAD_LIMIT; i += 1000) {
@@ -238,7 +239,7 @@ describe('DcsParser', () => {
238239
});
239240
it('should abort for payload limit +1', function(): void {
240241
this.timeout(10000);
241-
parser.setHandler(identifier({intermediates: '+', final: 'p'}), new DcsHandler((data, params) => reports.push([params.toArray(), data])));
242+
parser.registerHandler(identifier({intermediates: '+', final: 'p'}), new DcsHandler((data, params) => { reports.push([params.toArray(), data]); return true; }));
242243
parser.hook(identifier({intermediates: '+', final: 'p'}), Params.fromArray([1, 2, 3]));
243244
let data = toUtf32('A'.repeat(1000));
244245
for (let i = 0; i < PAYLOAD_LIMIT; i += 1000) {

src/common/parser/DcsParser.ts

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,10 @@ export class DcsParser implements IDcsParser {
2020
public dispose(): void {
2121
this._handlers = Object.create(null);
2222
this._handlerFb = () => {};
23+
this._active = EMPTY_HANDLERS;
2324
}
2425

25-
public addHandler(ident: number, handler: IDcsHandler): IDisposable {
26+
public registerHandler(ident: number, handler: IDcsHandler): IDisposable {
2627
if (this._handlers[ident] === undefined) {
2728
this._handlers[ident] = [];
2829
}
@@ -38,10 +39,6 @@ export class DcsParser implements IDcsParser {
3839
};
3940
}
4041

41-
public setHandler(ident: number, handler: IDcsHandler): void {
42-
this._handlers[ident] = [handler];
43-
}
44-
4542
public clearHandler(ident: number): void {
4643
if (this._handlers[ident]) delete this._handlers[ident];
4744
}
@@ -88,7 +85,7 @@ export class DcsParser implements IDcsParser {
8885
} else {
8986
let j = this._active.length - 1;
9087
for (; j >= 0; j--) {
91-
if (this._active[j].unhook(success) !== false) {
88+
if (this._active[j].unhook(success)) {
9289
break;
9390
}
9491
}
@@ -103,19 +100,27 @@ export class DcsParser implements IDcsParser {
103100
}
104101
}
105102

103+
// predefine empty params as [0] (ZDM)
104+
const EMPTY_PARAMS = new Params();
105+
EMPTY_PARAMS.addParam(0);
106+
106107
/**
107108
* Convenient class to create a DCS handler from a single callback function.
108109
* Note: The payload is currently limited to 50 MB (hardcoded).
109110
*/
110111
export class DcsHandler implements IDcsHandler {
111112
private _data = '';
112-
private _params: IParams | undefined;
113+
private _params: IParams = EMPTY_PARAMS;
113114
private _hitLimit: boolean = false;
114115

115-
constructor(private _handler: (data: string, params: IParams) => any) {}
116+
constructor(private _handler: (data: string, params: IParams) => boolean) {}
116117

117118
public hook(params: IParams): void {
118-
this._params = params.clone();
119+
// since we need to preserve params until `unhook`, we have to clone it
120+
// (only borrowed from parser and spans multiple parser states)
121+
// perf optimization:
122+
// clone only, if we have non empty params, otherwise stick with default
123+
this._params = (params.length > 1 || params.params[0]) ? params.clone() : EMPTY_PARAMS;
119124
this._data = '';
120125
this._hitLimit = false;
121126
}
@@ -131,14 +136,14 @@ export class DcsHandler implements IDcsHandler {
131136
}
132137
}
133138

134-
public unhook(success: boolean): any {
135-
let ret;
139+
public unhook(success: boolean): boolean {
140+
let ret = false;
136141
if (this._hitLimit) {
137142
ret = false;
138143
} else if (success) {
139-
ret = this._handler(this._data, this._params || new Params());
144+
ret = this._handler(this._data, this._params);
140145
}
141-
this._params = undefined;
146+
this._params = EMPTY_PARAMS;
142147
this._data = '';
143148
this._hitLimit = false;
144149
return ret;

0 commit comments

Comments
 (0)