From a972955c0ba11c13bcfa6f0be85813cc95013822 Mon Sep 17 00:00:00 2001 From: Denys Otrishko Date: Fri, 16 Jun 2017 20:26:43 +0300 Subject: [PATCH 1/3] lib: optimize connection events * remove unnecessary events like 'handshake', 'call', 'handshakeRequest' * replace 'callbackForUnknownPacket' with rejectPacket * change signature of connection 'event' (remote event), now it contains interfaceName, eventName, eventArgs Fixes: https://github.com/metarhia/jstp/issues/49 --- lib/connection.js | 81 +++++------------------------------ test/node/connection-event.js | 34 ++++++++------- 2 files changed, 28 insertions(+), 87 deletions(-) diff --git a/lib/connection.js b/lib/connection.js index f174cead..080654f1 100644 --- a/lib/connection.js +++ b/lib/connection.js @@ -58,6 +58,10 @@ function Connection(transport, server, client) { this._heartbeatCallbackInstance = null; + this._emitError = (error) => { + if (error) this.emit('error', error); + }; + transport.on('packet', this._processPacket.bind(this)); transport.on('close', this._onSocketClose.bind(this)); transport.on('error', this._onSocketError.bind(this)); @@ -77,9 +81,7 @@ Connection.prototype.callMethod = function( ) { const packet = this.createPacket('call', interfaceName, methodName, args); const packetId = packet.call[0]; - this._callbacks[packetId] = callback || ((error) => { - if (error) this.emit('error', error); - }); + this._callbacks[packetId] = callback || this._emitError; this._send(packet); }; @@ -393,12 +395,6 @@ Connection.prototype._processHandshakeRequest = function(packet, keys) { const credentials = authStrategy && packet[authStrategy]; authStrategy = authStrategy || 'anonymous'; - this._emitPacketEvent('handshakeRequest', packet, packet.handshake[0], { - packetType: 'handshake', - handshakeRequest: true, - authStrategy - }); - this.server.startSession(this, application, authStrategy, credentials, this._onSessionCreated.bind(this)); }; @@ -442,10 +438,6 @@ Connection.prototype._processHandshakeResponse = function(packet) { this.handshakeDone = true; this.application = this.client.application; - this._emitPacketEvent('handshake', packet, packetId, { - sessionId: packet.ok - }); - callback(null, packet.ok); } else if (packet.error) { delete this._callbacks[packetId]; @@ -475,11 +467,6 @@ Connection.prototype._processCallPacket = function(packet, keys) { const methodName = keys[1]; const args = packet[methodName]; - this._emitPacketEvent('call', packet, packetId, { - interfaceName, - methodName - }); - const callback = this._remoteCallbackWrapper.bind(this, packetId); if (!args) { @@ -506,20 +493,12 @@ Connection.prototype._processCallbackPacket = function(packet) { delete this._callbacks[packetId]; if (packet.ok) { - callback(null, ...packet.ok); + return callback(null, ...packet.ok); } else if (packet.error) { - callback(errors.RemoteError.fromJstpArray(packet.error)); - } else { - this._rejectPacket(packet); + return callback(errors.RemoteError.fromJstpArray(packet.error)); } } - - const eventArgs = callback ? null : { sourcePacketUnknown: true }; - this._emitPacketEvent('callback', packet, packetId, eventArgs); - - if (!callback) { - this.emit('callbackForUnknownPacket', packetId, packet, this); - } + this._rejectPacket(packet); }; // Process incoming event packet @@ -531,11 +510,7 @@ Connection.prototype._processEventPacket = function(packet, keys) { const eventName = keys[1]; const eventArgs = packet[eventName]; - this._emitPacketEvent('event', packet, packet.event[0], { - interfaceName, - remoteEventName: eventName, - remoteEventArgs: eventArgs - }); + this.emit('event', interfaceName, eventName, eventArgs); const remoteProxy = this.remoteProxies[interfaceName]; if (remoteProxy) { @@ -550,8 +525,6 @@ Connection.prototype._processInspectPacket = function(packet) { const packetId = packet.inspect[0]; const interfaceName = packet.inspect[1]; - this._emitPacketEvent('inspect', packet, packetId, { interfaceName }); - const methods = this.application.getMethods(interfaceName); if (methods) { this.callback(packetId, null, methods); @@ -561,17 +534,8 @@ Connection.prototype._processInspectPacket = function(packet) { }; // Process incoming state packet -// packet - parsed packet // -Connection.prototype._processStatePacket = function(packet, keys) { - const path = packet.state[1]; - const verb = keys[1]; - const value = packet[verb]; - - this._emitPacketEvent('state', packet, packet.state[0], { - path, verb, value - }); -}; +Connection.prototype._processStatePacket = function() {}; // Process incoming ping packet // packet - parsed packet @@ -604,31 +568,6 @@ Connection.prototype._remoteCallbackWrapper = function( this.callback(packetId, error, result); }; -// Emit an event notifying about incoming packet. The event payload is an -// object that contains information about the connection, application, packet, -// packet type, packet ID and any additional data that you pass to this -// function. -// kind - packet type and event name -// packet - parsed packet -// packetId - packet ID -// args - additional event arguments (optional) -// -Connection.prototype._emitPacketEvent = function(kind, packet, packetId, args) { - const eventArgs = { - connection: this, - packetType: kind, - packet, - packetId, - application: this.application - }; - - if (args) { - Object.assign(eventArgs, args); - } - - this.emit(kind, eventArgs); -}; - PACKET_HANDLERS = { handshake: Connection.prototype._processHandshakePacket, call: Connection.prototype._processCallPacket, diff --git a/test/node/connection-event.js b/test/node/connection-event.js index e449f490..e54213b1 100644 --- a/test/node/connection-event.js +++ b/test/node/connection-event.js @@ -42,16 +42,17 @@ test.test('server must process an event', (test) => { client.connectAndHandshake(app.name, null, null, (error, connection) => { test.assertNot(error, 'must connect to server'); - server.getClients()[0].on('event', (event) => { - test.strictEqual(event.interfaceName, iface, + server.getClients()[0].on('event', + (interfaceName, remoteName, remoteArgs) => { + test.strictEqual(interfaceName, iface, 'event interface must match'); - test.strictEqual(event.remoteEventName, eventName, + test.strictEqual(remoteName, eventName, 'event name must be equal to the emitted one'); - test.strictDeepEqual(event.remoteEventArgs, args, + test.strictDeepEqual(remoteArgs, args, 'event arguments must be equal to the passed ones'); - test.end(); - }); + test.end(); + }); connection.emitRemoteEvent(iface, eventName, args); }); @@ -61,12 +62,12 @@ test.test('client must process an event', (test) => { client.connectAndHandshake(app.name, null, null, (error, connection) => { test.assertNot(error, 'must connect to server'); - connection.on('event', (event) => { - test.strictEqual(event.interfaceName, iface, + connection.on('event', (interfaceName, remoteName, remoteArgs) => { + test.strictEqual(interfaceName, iface, 'event interface must match'); - test.strictEqual(event.remoteEventName, eventName, + test.strictEqual(remoteName, eventName, 'event name must be equal to the emitted one'); - test.strictDeepEqual(event.remoteEventArgs, args, + test.strictDeepEqual(remoteArgs, args, 'event arguments must be equal to the passed ones'); test.end(); }); @@ -80,15 +81,16 @@ test.test('remote proxy must emit an event', (test) => { (error, connection, api) => { test.assertNot(error, 'must connect to server'); - server.getClients()[0].on('event', (event) => { - test.strictEqual(event.interfaceName, iface, + server.getClients()[0].on('event', + (interfaceName, remoteName, remoteArgs) => { + test.strictEqual(interfaceName, iface, 'event interface must match'); - test.strictEqual(event.remoteEventName, eventName, + test.strictEqual(remoteName, eventName, 'event name must be equal to the emitted one'); - test.strictDeepEqual(event.remoteEventArgs, args, + test.strictDeepEqual(remoteArgs, args, 'event arguments must be equal to the passed ones'); - test.end(); - }); + test.end(); + }); api.iface.emit(eventName, ...args); } From 1789dd8aa2d540149d4d7d721e5b0f5372832000 Mon Sep 17 00:00:00 2001 From: Denys Otrishko Date: Mon, 19 Jun 2017 15:11:41 +0300 Subject: [PATCH 2/3] fixup! lib: remove state packets --- lib/connection.js | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/lib/connection.js b/lib/connection.js index 080654f1..5eb21be6 100644 --- a/lib/connection.js +++ b/lib/connection.js @@ -117,17 +117,6 @@ Connection.prototype.emitRemoteEvent = function( this._send(packet); }; -// Send a state synchronization packet over the connection -// path - path to a value in the application state -// verb - operation with this value (inc, dec, let, delete, push, pop, shift, -// unshift) -// value - a value to modify the current value with -// -Connection.prototype.notifyStateChange = function(path, verb, value) { - const packet = this.createPacket('state', path, verb, value); - this._send(packet); -}; - // Send a handshake packet over the connection // appName - name of an application to connect to // login - user name (optional) @@ -533,10 +522,6 @@ Connection.prototype._processInspectPacket = function(packet) { } }; -// Process incoming state packet -// -Connection.prototype._processStatePacket = function() {}; - // Process incoming ping packet // packet - parsed packet // @@ -574,7 +559,6 @@ PACKET_HANDLERS = { callback: Connection.prototype._processCallbackPacket, event: Connection.prototype._processEventPacket, inspect: Connection.prototype._processInspectPacket, - state: Connection.prototype._processStatePacket, ping: Connection.prototype._processPingPacket, pong: Connection.prototype._processPongPacket }; From 91087d3d1477d2b08fda2c93cddd02a8d5022af5 Mon Sep 17 00:00:00 2001 From: Denys Otrishko Date: Tue, 20 Jun 2017 15:01:52 +0300 Subject: [PATCH 3/3] fixup! add requested changes --- lib/connection.js | 2 ++ test/node/connection-event.js | 12 ++++++------ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/connection.js b/lib/connection.js index 5eb21be6..5097933c 100644 --- a/lib/connection.js +++ b/lib/connection.js @@ -58,6 +58,8 @@ function Connection(transport, server, client) { this._heartbeatCallbackInstance = null; + // Defined in constructor to be used as default callback in callMethod + // without binding it. this._emitError = (error) => { if (error) this.emit('error', error); }; diff --git a/test/node/connection-event.js b/test/node/connection-event.js index e54213b1..70a77ba4 100644 --- a/test/node/connection-event.js +++ b/test/node/connection-event.js @@ -45,11 +45,11 @@ test.test('server must process an event', (test) => { server.getClients()[0].on('event', (interfaceName, remoteName, remoteArgs) => { test.strictEqual(interfaceName, iface, - 'event interface must match'); + 'event interface must match'); test.strictEqual(remoteName, eventName, - 'event name must be equal to the emitted one'); + 'event name must be equal to the emitted one'); test.strictDeepEqual(remoteArgs, args, - 'event arguments must be equal to the passed ones'); + 'event arguments must be equal to the passed ones'); test.end(); }); @@ -84,11 +84,11 @@ test.test('remote proxy must emit an event', (test) => { server.getClients()[0].on('event', (interfaceName, remoteName, remoteArgs) => { test.strictEqual(interfaceName, iface, - 'event interface must match'); + 'event interface must match'); test.strictEqual(remoteName, eventName, - 'event name must be equal to the emitted one'); + 'event name must be equal to the emitted one'); test.strictDeepEqual(remoteArgs, args, - 'event arguments must be equal to the passed ones'); + 'event arguments must be equal to the passed ones'); test.end(); });