From 38f89ef997f24636ea3f9412fa8a5de3c1eb41ed Mon Sep 17 00:00:00 2001 From: jahorton Date: Fri, 17 Apr 2020 09:48:55 +0700 Subject: [PATCH] refactor(common/lmlayer): restores original constructor form, drops factories --- common/predictive-text/index.ts | 6 ++-- common/predictive-text/lmlayer-interface.ts | 6 ++-- .../unit_tests/headless/top-level-lmlayer.js | 34 ++++++++----------- ...-workerFactory.ts => web-defaultWorker.ts} | 8 ++--- common/predictive-text/workerFactory.ts | 5 --- web/source/includes/lmlayer.ts | 1 + 6 files changed, 23 insertions(+), 37 deletions(-) rename common/predictive-text/{web-workerFactory.ts => web-defaultWorker.ts} (78%) delete mode 100644 common/predictive-text/workerFactory.ts diff --git a/common/predictive-text/index.ts b/common/predictive-text/index.ts index 3aa1c61117..93e2ada8ba 100644 --- a/common/predictive-text/index.ts +++ b/common/predictive-text/index.ts @@ -23,7 +23,6 @@ /// /// /// -/// /** * Top-level interface to the Language Modelling layer, or "LMLayer" for short. @@ -53,8 +52,9 @@ namespace com.keyman.text.prediction { * @param uri URI of the underlying LMLayer worker code. This will usually be a blob: * or file: URI. If uri is not provided, this will start the default Worker. */ - constructor(capabilities: Capabilities) { - super(capabilities, new WebWorkerFactory()); + constructor(capabilities: Capabilities, worker?: Worker) { + worker = worker || DefaultWorker.constructInstance(); + super(capabilities, worker); } } } diff --git a/common/predictive-text/lmlayer-interface.ts b/common/predictive-text/lmlayer-interface.ts index 47a8a8d5c5..18a47b5551 100644 --- a/common/predictive-text/lmlayer-interface.ts +++ b/common/predictive-text/lmlayer-interface.ts @@ -1,5 +1,3 @@ -/// - namespace com.keyman.text.prediction { export abstract class LMLayerBase { /** @@ -19,9 +17,9 @@ namespace com.keyman.text.prediction { * @param uri URI of the underlying LMLayer worker code. This will usually be a blob: * or file: URI. If uri is not provided, this will start the default Worker. */ - constructor(capabilities: Capabilities, workerFactory: WorkerFactory) { + constructor(capabilities: Capabilities, worker: Worker) { // Either use the given worker, or instantiate the default worker. - this._worker = workerFactory.constructInstance(); + this._worker = worker; this._worker.onmessage = this.onMessage.bind(this) this._declareLMLayerReady = null; this._predictPromises = new PromiseStore; diff --git a/common/predictive-text/unit_tests/headless/top-level-lmlayer.js b/common/predictive-text/unit_tests/headless/top-level-lmlayer.js index de8351fd10..dfad22e730 100644 --- a/common/predictive-text/unit_tests/headless/top-level-lmlayer.js +++ b/common/predictive-text/unit_tests/headless/top-level-lmlayer.js @@ -9,14 +9,14 @@ let LMLayerBase = LMLayer.LMLayerBase; describe('LMLayer', function() { describe('[[constructor]]', function () { it('should accept a Worker to instantiate', function () { - new LMLayerBase(capabilities(), createFakeWorkerFactory()); + new LMLayerBase(capabilities(), createFakeWorker()); }); it('should send the `config` message to the LMLayer', async function () { - let fakeWorker = createFakeWorkerFactory(fakePostMessage); + let fakeWorker = createFakeWorker(fakePostMessage); let lmLayer = new LMLayerBase(capabilities(), fakeWorker); - assert.propertyVal(fakeWorker.instance.postMessage, 'callCount', 1); + assert.propertyVal(fakeWorker.postMessage, 'callCount', 1); // In the "Worker", assert the message looks right function fakePostMessage(data) { assert.propertyVal(data, 'message', 'config'); @@ -27,20 +27,20 @@ describe('LMLayer', function() { describe('#loadModel()', function () { it('should accept capabilities and model description', function () { - let fakeWorker = createFakeWorkerFactory(); + let fakeWorker = createFakeWorker(); let lmLayer = new LMLayerBase(capabilities(), fakeWorker); lmLayer.loadModel("./unit_tests/in_browser/resources/models/simple-dummy.js"); - assert.isFunction(fakeWorker.instance.onmessage, 'LMLayer failed to set a callback!'); + assert.isFunction(fakeWorker.onmessage, 'LMLayer failed to set a callback!'); }); it('should send the `load` message to the LMLayer', async function () { - let fakeWorker = createFakeWorkerFactory(fakePostMessage); + let fakeWorker = createFakeWorker(fakePostMessage); let lmLayer = new LMLayerBase(capabilities(), fakeWorker); let configuration = await lmLayer.loadModel("./unit_tests/in_browser/resources/models/simple-dummy.js"); - assert.propertyVal(fakeWorker.instance.postMessage, 'callCount', 2); + assert.propertyVal(fakeWorker.postMessage, 'callCount', 2); // In the "Worker", assert the message looks right and // ASYNCHRONOUSLY reply with ready message. function fakePostMessage(data) { @@ -52,7 +52,7 @@ describe('LMLayer', function() { assert.propertyVal(data, 'message', 'load'); assert.isString(data.model); - callAsynchronously(() => fakeWorker.instance.onmessage({ + callAsynchronously(() => fakeWorker.onmessage({ data: { message: 'ready', configuration: {} @@ -67,8 +67,8 @@ describe('LMLayer', function() { rightContextCodeUnits: 0, } - let fakeWorker = createFakeWorkerFactory(function fakePostMessage(_data) { - callAsynchronously(() => fakeWorker.instance.onmessage({ + let fakeWorker = createFakeWorker(function fakePostMessage(_data) { + callAsynchronously(() => fakeWorker.onmessage({ data: { message: 'ready', configuration: expectedConfiguration @@ -114,17 +114,11 @@ describe('LMLayer', function() { * * @returns {WorkerFactory} a 'factory' class returning a Worker object with sinon.fake() instances. */ - function createFakeWorkerFactory(postMessage) { - let FakeFactory = function() {}; // barebones JS class definition - requires function as core, constructor. - FakeFactory.prototype.constructInstance = function() { - this.instance = { - postMessage: postMessage ? sinon.fake(postMessage) : sinon.fake(), - onmessage: null - }; - - return this.instance; + function createFakeWorker(postMessage) { + return { + postMessage: postMessage ? sinon.fake(postMessage) : sinon.fake(), + onmessage: null }; - return new FakeFactory(); } /** diff --git a/common/predictive-text/web-workerFactory.ts b/common/predictive-text/web-defaultWorker.ts similarity index 78% rename from common/predictive-text/web-workerFactory.ts rename to common/predictive-text/web-defaultWorker.ts index aa230d82e7..6a4f527c13 100644 --- a/common/predictive-text/web-workerFactory.ts +++ b/common/predictive-text/web-defaultWorker.ts @@ -1,9 +1,7 @@ -/// - namespace com.keyman.text.prediction { - export class WebWorkerFactory implements WorkerFactory { - constructInstance(): Worker { - return new Worker(WebWorkerFactory.asBlobURI(LMLayerWorkerCode)); + export class DefaultWorker { + static constructInstance(): Worker { + return new Worker(this.asBlobURI(LMLayerWorkerCode)); } /** diff --git a/common/predictive-text/workerFactory.ts b/common/predictive-text/workerFactory.ts deleted file mode 100644 index f80e820364..0000000000 --- a/common/predictive-text/workerFactory.ts +++ /dev/null @@ -1,5 +0,0 @@ -namespace com.keyman.text.prediction { - export interface WorkerFactory { - constructInstance(): Worker; - } -} \ No newline at end of file diff --git a/web/source/includes/lmlayer.ts b/web/source/includes/lmlayer.ts index e34f8c6d21..5a60c03e8f 100644 --- a/web/source/includes/lmlayer.ts +++ b/web/source/includes/lmlayer.ts @@ -4,6 +4,7 @@ // Defines the main interface of the Language Modeling Layer (LMLayer) and its original typing information. /// +/// /// // We DO need the embedded_worker.d.ts file - since we're directly linking into the LMLayer's code,