From 69ebf017ffcc157fb6b9407d1f9673412099968b Mon Sep 17 00:00:00 2001 From: Beaudan Date: Wed, 6 Feb 2019 09:45:11 +1100 Subject: [PATCH 1/4] Reworked loki_p2p_api to not use the window object, which means it needs to be instantiated after the storage is ready so that your pubkey can be passed in to the constructor. This makes it more modular and allows for easier testing --- js/background.js | 7 +++++++ js/modules/loki_p2p_api.js | 19 +++++++++++++------ preload.js | 4 +--- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/js/background.js b/js/background.js index 50ff93c31..b25fc5d41 100644 --- a/js/background.js +++ b/js/background.js @@ -219,6 +219,13 @@ return; } first = false; + window.lokiP2pAPI = new window.LokiP2pAPI( + textsecure.storage.user.getNumber() + ); + window.lokiP2pAPI.on( + 'pingContact', + window.libloki.api.sendOnlineBroadcastMessage + ); // These make key operations available to IPC handlers created in preload.js window.Events = { diff --git a/js/modules/loki_p2p_api.js b/js/modules/loki_p2p_api.js index 6fc18f10d..f8d9563d9 100644 --- a/js/modules/loki_p2p_api.js +++ b/js/modules/loki_p2p_api.js @@ -1,16 +1,23 @@ -/* global setTimeout, clearTimeout, window */ +/* global setTimeout, clearTimeout */ const EventEmitter = require('events'); class LokiP2pAPI extends EventEmitter { - constructor() { + constructor(ourKey) { super(); this.contactP2pDetails = {}; + this.ourKey = ourKey; } - updateContactP2pDetails(pubKey, address, port, fromP2p = false) { + reset() { + Object.keys(this.contactP2pDetails).forEach(key => { + clearTimeout(this.contactP2pDetails[key].pingTimer); + delete this.contactP2pDetails[key]; + }); + } + + updateContactP2pDetails(pubKey, address, port, isOnline = false) { // Stagger the timers so the friends don't ping each other at the same time - this.ourKey = this.ourKey || window.textsecure.storage.user.getNumber(); const timerDuration = pubKey < this.ourKey ? 60 * 1000 // 1 minute @@ -28,7 +35,7 @@ class LokiP2pAPI extends EventEmitter { pingTimer: null, }; - if (fromP2p) { + if (isOnline) { this.setContactOnline(pubKey); return; } @@ -73,7 +80,7 @@ class LokiP2pAPI extends EventEmitter { if (!this.contactP2pDetails[pubKey]) { return; } - window.libloki.api.sendOnlineBroadcastMessage(pubKey, true); + this.emit('pingContact', pubKey, true); } } diff --git a/preload.js b/preload.js index fb3dc298a..1e2fd0468 100644 --- a/preload.js +++ b/preload.js @@ -276,9 +276,7 @@ window.LokiSnodeAPI = new LokiSnodeAPI({ swarmServerPort: config.swarmServerPort, }); -const LokiP2pAPI = require('./js/modules/loki_p2p_api'); - -window.lokiP2pAPI = new LokiP2pAPI(); +window.LokiP2pAPI = require('./js/modules/loki_p2p_api'); const LokiMessageAPI = require('./js/modules/loki_message_api'); From 02d6920ade9beb658ea187057d310e69789bf240 Mon Sep 17 00:00:00 2001 From: Beaudan Date: Wed, 6 Feb 2019 10:25:37 +1100 Subject: [PATCH 2/4] Tests for loki_p2p_api, added yarn command to generate coverage html, instantiating loki_p2p_api in the test preload --- libloki/test/node/loki_p2p_api_test.js | 112 +++++++++++++++++++++++++ package.json | 1 + test/_test.js | 2 + 3 files changed, 115 insertions(+) create mode 100644 libloki/test/node/loki_p2p_api_test.js diff --git a/libloki/test/node/loki_p2p_api_test.js b/libloki/test/node/loki_p2p_api_test.js new file mode 100644 index 000000000..1c4d261ce --- /dev/null +++ b/libloki/test/node/loki_p2p_api_test.js @@ -0,0 +1,112 @@ +const { assert } = require('chai'); +const LokiP2pAPI = require('../../../js/modules/loki_p2p_api'); + +describe('LocalLokiServer', () => { + const usedKey = 'aPubKey'; + const usedAddress = 'anAddress'; + const usedPort = 'aPort'; + + beforeEach(() => { + this.lokiP2pAPI = new LokiP2pAPI(); + }); + + afterEach(() => { + this.lokiP2pAPI.reset(); + }); + + it("Should not emit a pingContact event if that contact doesn't exits", async () => { + this.lokiP2pAPI.on('pingContact', () => { + assert.fail(); + }); + this.lokiP2pAPI.pingContact('not stored'); + }); + + it('Should emit an online event if the contact is online', async () => { + let promise; + const timer = setTimeout(() => { + promise = Promise.resolve(); + }, 5000); + this.lokiP2pAPI.on('online', pubKey => { + clearTimeout(timer); + promise = Promise.resolve(pubKey); + }); + this.lokiP2pAPI.updateContactP2pDetails( + usedKey, + usedAddress, + usedPort, + true + ); + assert.strictEqual(await promise, usedKey); + }); + + it("Should send a pingContact event if the contact isn't online", async () => { + let promise; + const timer = setTimeout(() => { + promise = Promise.resolve(); + }, 5000); + this.lokiP2pAPI.on('pingContact', (pubKey, forceP2p) => { + assert.isTrue(forceP2p); + clearTimeout(timer); + promise = Promise.resolve(pubKey); + }); + this.lokiP2pAPI.updateContactP2pDetails( + usedKey, + usedAddress, + usedPort, + false + ); + assert.strictEqual(await promise, usedKey); + }); + + it('Should store a contacts p2p details', async () => { + let promise; + const timer = setTimeout(() => { + promise = Promise.resolve(); + }, 5000); + this.lokiP2pAPI.on('online', pubKey => { + clearTimeout(timer); + promise = Promise.resolve(pubKey); + }); + this.lokiP2pAPI.updateContactP2pDetails( + usedKey, + usedAddress, + usedPort, + true + ); + await promise; + const p2pDetails = this.lokiP2pAPI.getContactP2pDetails(usedKey); + assert.strictEqual(usedAddress, p2pDetails.address); + assert.strictEqual(usedPort, p2pDetails.port); + }); + + it('Should say if a contact is online', async () => { + this.lokiP2pAPI.updateContactP2pDetails( + usedKey, + usedAddress, + usedPort, + true + ); + assert.isTrue(this.lokiP2pAPI.isOnline(usedKey)); + this.lokiP2pAPI.updateContactP2pDetails( + usedKey, + usedAddress, + usedPort, + false + ); + assert.isFalse(this.lokiP2pAPI.isOnline(usedKey)); + }); + + it('Should set a contact as offline', async () => { + this.lokiP2pAPI.updateContactP2pDetails( + usedKey, + usedAddress, + usedPort, + true + ); + let p2pDetails = this.lokiP2pAPI.getContactP2pDetails(usedKey); + assert.isTrue(p2pDetails.isOnline); + p2pDetails = this.lokiP2pAPI.getContactP2pDetails(usedKey); + this.lokiP2pAPI.setContactOffline(usedKey); + assert.isFalse(p2pDetails.isOnline); + }); +}); diff --git a/package.json b/package.json index a4f6d8898..b7d9092ed 100644 --- a/package.json +++ b/package.json @@ -33,6 +33,7 @@ "test-electron": "yarn grunt test", "test-node": "mocha --recursive test/app test/modules ts/test libloki/test/node", "test-node-coverage": "nyc --reporter=lcov --reporter=text mocha --recursive test/app test/modules ts/test libloki/test/node", + "test-node-coverage-html": "nyc --reporter=lcov --reporter=html mocha --recursive test/app test/modules ts/test libloki/test/node", "eslint": "eslint .", "lint": "yarn format --list-different && yarn lint-windows", "lint-windows": "yarn eslint && yarn tslint", diff --git a/test/_test.js b/test/_test.js index fd9f8fcc2..f55ff8bc2 100644 --- a/test/_test.js +++ b/test/_test.js @@ -79,3 +79,5 @@ before(async () => { window.clearDatabase = async () => { await window.Signal.Data.removeAll(); }; + +window.lokiP2pAPI = new window.LokiP2pAPI('ourKey'); From cde7bbf3c3a6beb25c0d81dbde168af985a40de2 Mon Sep 17 00:00:00 2001 From: Beaudan Date: Wed, 6 Feb 2019 11:49:46 +1100 Subject: [PATCH 3/4] Removed forceP2p as argument for the pingContact event and enforced it on the other side --- js/background.js | 8 ++++---- js/modules/loki_p2p_api.js | 2 +- libloki/test/node/loki_p2p_api_test.js | 3 +-- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/js/background.js b/js/background.js index b25fc5d41..f37fc6fd7 100644 --- a/js/background.js +++ b/js/background.js @@ -222,10 +222,10 @@ window.lokiP2pAPI = new window.LokiP2pAPI( textsecure.storage.user.getNumber() ); - window.lokiP2pAPI.on( - 'pingContact', - window.libloki.api.sendOnlineBroadcastMessage - ); + window.lokiP2pAPI.on('pingContact', pubKey => { + const forceP2p = true; + window.libloki.api.sendOnlineBroadcastMessage(pubKey, forceP2p); + }); // These make key operations available to IPC handlers created in preload.js window.Events = { diff --git a/js/modules/loki_p2p_api.js b/js/modules/loki_p2p_api.js index f8d9563d9..349517c6b 100644 --- a/js/modules/loki_p2p_api.js +++ b/js/modules/loki_p2p_api.js @@ -80,7 +80,7 @@ class LokiP2pAPI extends EventEmitter { if (!this.contactP2pDetails[pubKey]) { return; } - this.emit('pingContact', pubKey, true); + this.emit('pingContact', pubKey); } } diff --git a/libloki/test/node/loki_p2p_api_test.js b/libloki/test/node/loki_p2p_api_test.js index 1c4d261ce..9448be27c 100644 --- a/libloki/test/node/loki_p2p_api_test.js +++ b/libloki/test/node/loki_p2p_api_test.js @@ -44,8 +44,7 @@ describe('LocalLokiServer', () => { const timer = setTimeout(() => { promise = Promise.resolve(); }, 5000); - this.lokiP2pAPI.on('pingContact', (pubKey, forceP2p) => { - assert.isTrue(forceP2p); + this.lokiP2pAPI.on('pingContact', pubKey => { clearTimeout(timer); promise = Promise.resolve(pubKey); }); From 25a3129e3781910fa3524e0612ed6404e542f986 Mon Sep 17 00:00:00 2001 From: Beaudan Date: Wed, 6 Feb 2019 12:01:59 +1100 Subject: [PATCH 4/4] Reworked p2p tests to be sensible and use the built in timeout/done system instead of hacky promise thing --- libloki/test/node/loki_p2p_api_test.js | 43 +++++++------------------- 1 file changed, 12 insertions(+), 31 deletions(-) diff --git a/libloki/test/node/loki_p2p_api_test.js b/libloki/test/node/loki_p2p_api_test.js index 9448be27c..5875927ea 100644 --- a/libloki/test/node/loki_p2p_api_test.js +++ b/libloki/test/node/loki_p2p_api_test.js @@ -14,21 +14,17 @@ describe('LocalLokiServer', () => { this.lokiP2pAPI.reset(); }); - it("Should not emit a pingContact event if that contact doesn't exits", async () => { + it("Should not emit a pingContact event if that contact doesn't exits", () => { this.lokiP2pAPI.on('pingContact', () => { assert.fail(); }); this.lokiP2pAPI.pingContact('not stored'); }); - it('Should emit an online event if the contact is online', async () => { - let promise; - const timer = setTimeout(() => { - promise = Promise.resolve(); - }, 5000); + it('Should emit an online event if the contact is online', done => { this.lokiP2pAPI.on('online', pubKey => { - clearTimeout(timer); - promise = Promise.resolve(pubKey); + assert.strictEqual(pubKey, usedKey); + done(); }); this.lokiP2pAPI.updateContactP2pDetails( usedKey, @@ -36,17 +32,12 @@ describe('LocalLokiServer', () => { usedPort, true ); - assert.strictEqual(await promise, usedKey); - }); + }).timeout(1000); - it("Should send a pingContact event if the contact isn't online", async () => { - let promise; - const timer = setTimeout(() => { - promise = Promise.resolve(); - }, 5000); + it("Should send a pingContact event if the contact isn't online", done => { this.lokiP2pAPI.on('pingContact', pubKey => { - clearTimeout(timer); - promise = Promise.resolve(pubKey); + assert.strictEqual(pubKey, usedKey); + done(); }); this.lokiP2pAPI.updateContactP2pDetails( usedKey, @@ -54,31 +45,21 @@ describe('LocalLokiServer', () => { usedPort, false ); - assert.strictEqual(await promise, usedKey); - }); + }).timeout(1000); - it('Should store a contacts p2p details', async () => { - let promise; - const timer = setTimeout(() => { - promise = Promise.resolve(); - }, 5000); - this.lokiP2pAPI.on('online', pubKey => { - clearTimeout(timer); - promise = Promise.resolve(pubKey); - }); + it('Should store a contacts p2p details', () => { this.lokiP2pAPI.updateContactP2pDetails( usedKey, usedAddress, usedPort, true ); - await promise; const p2pDetails = this.lokiP2pAPI.getContactP2pDetails(usedKey); assert.strictEqual(usedAddress, p2pDetails.address); assert.strictEqual(usedPort, p2pDetails.port); }); - it('Should say if a contact is online', async () => { + it('Should say if a contact is online', () => { this.lokiP2pAPI.updateContactP2pDetails( usedKey, usedAddress, @@ -95,7 +76,7 @@ describe('LocalLokiServer', () => { assert.isFalse(this.lokiP2pAPI.isOnline(usedKey)); }); - it('Should set a contact as offline', async () => { + it('Should set a contact as offline', () => { this.lokiP2pAPI.updateContactP2pDetails( usedKey, usedAddress,