Repository: moscajs/aedes
Pull request #456 (2020-03-03 12:53:03)

Expose protocol version in Client object
as title, we expose `CONNECT` protocol version into Client that add flexibility to our users to fine-tune their hook handlers.

robertsLando: @gnought Could you merge this?
gnought: Sure

Patch:
diff --git a/aedes.d.ts b/aedes.d.ts
index 9c39601..be8f274 100644
--- a/aedes.d.ts
+++ b/aedes.d.ts
@@ -74,6 +74,7 @@ declare namespace aedes {
   interface Client extends EventEmitter {
     id: string
     clean: boolean
+    version: number
     conn: Connection
     req?: IncomingMessage
     connecting: boolean
diff --git a/docs/Client.md b/docs/Client.md
index ea95383..f0fd3df 100644
--- a/docs/Client.md
+++ b/docs/Client.md
@@ -62,13 +62,25 @@ a read-only flag indicates if client is closed or not.
 
 - `<string>` __Default__: `aedes_${shortid()}`
 
-Client unique identifier, specified by CONNECT packet. It is `null` in [`aedes.preConnect`](./Aedes.md#handler-preconnect-client-callback) unless the handler passes.
+Client unique identifier, specified by CONNECT packet.
+
+It is available only after `CONNACK (rc=0)`, otherwise it is `null` in cases:
+
+- in [`aedes.preConnect`](./Aedes.md#handler-preconnect-client-callback) stage
+- after `CONNACK (rc!=0)` response
+- `connectionError` raised by aedes
 
 ## client.clean
 
 - `<boolean>` __Default__: `true`
 
-Client clean flag, specified by CONNECT packet.
+Client clean flag, set by clean flag in `CONNECT` packet.
+
+## client.version
+
+- `<number>` __Default__: `null`
+
+Client version, set by protocol version in `CONNECT` packet when `CONNACK (rc=0)` returns.
 
 ## Event: connected
 
diff --git a/lib/client.js b/lib/client.js
index d9f48f1..481e6b1 100644
--- a/lib/client.js
+++ b/lib/client.js
@@ -27,6 +27,7 @@ function Client (broker, conn, req) {
   // mqtt params
   this.id = null
   this.clean = true
+  this.version = null
 
   this.subscriptions = {}
   this.duplicates = {}
diff --git a/lib/handlers/connect.js b/lib/handlers/connect.js
index 093f26a..ff5bb41 100644
--- a/lib/handlers/connect.js
+++ b/lib/handlers/connect.js
@@ -81,6 +81,7 @@ function init (client, packet, done) {
 
   client.id = clientId || 'aedes_' + shortid()
   client.clean = packet.clean
+  client.version = packet.protocolVersion
   client._will = packet.will
 
   client.broker._series(


Test patch:
diff --git a/test/connect.js b/test/connect.js
index 8c1d443..853cd6c 100644
--- a/test/connect.js
+++ b/test/connect.js
@@ -13,7 +13,7 @@ const aedes = require('../')
 
 ;[{ ver: 3, id: 'MQIsdp' }, { ver: 4, id: 'MQTT' }].forEach(function (ele) {
   test('connect and connack (minimal)', function (t) {
-    t.plan(1)
+    t.plan(2)
 
     const s = setup()
     t.tearDown(s.broker.close.bind(s.broker))
@@ -39,6 +39,7 @@ const aedes = require('../')
         payload: null,
         sessionPresent: false
       }, 'successful connack')
+      t.equal(s.client.version, ele.ver)
     })
   })
 })
@@ -75,7 +76,7 @@ test('reject client requested for unacceptable protocol version', function (t) {
 
 // [MQTT-3.1.2-1], Guarded in mqtt-packet
 test('reject client requested for unsupported protocol version', function (t) {
-  t.plan(2)
+  t.plan(3)
 
   const broker = aedes()
   t.tearDown(broker.close.bind(broker))
@@ -94,6 +95,7 @@ test('reject client requested for unsupported protocol version', function (t) {
     t.fail('no data sent')
   })
   broker.on('connectionError', function (client, err) {
+    t.equal(client.version, null)
     t.equal(err.message, 'Invalid protocol version')
     t.equal(broker.connectedClients, 0)
   })
@@ -101,7 +103,7 @@ test('reject client requested for unsupported protocol version', function (t) {
 
 // Guarded in mqtt-packet
 test('reject clients with no clientId running on MQTT 3.1.0', function (t) {
-  t.plan(2)
+  t.plan(3)
 
   const broker = aedes()
   t.tearDown(broker.close.bind(broker))
@@ -119,6 +121,7 @@ test('reject clients with no clientId running on MQTT 3.1.0', function (t) {
     t.fail('no data sent')
   })
   broker.on('connectionError', function (client, err) {
+    t.equal(client.version, null)
     t.equal(err.message, 'clientId must be supplied before 3.1.1')
     t.equal(broker.connectedClients, 0)
   })
@@ -151,7 +154,7 @@ test('reject clients without clientid and clean=false on MQTT 3.1.1', function (
 })
 
 test('clients without clientid and clean=true on MQTT 3.1.1 will get a generated clientId', function (t) {
-  t.plan(4)
+  t.plan(5)
 
   const broker = aedes()
   t.tearDown(broker.close.bind(broker))
@@ -169,6 +172,7 @@ test('clients without clientid and clean=true on MQTT 3.1.1 will get a generated
     t.equal(packet.cmd, 'connack')
     t.equal(packet.returnCode, 0)
     t.equal(broker.connectedClients, 1)
+    t.equal(s.client.version, 4)
   })
   broker.on('connectionError', function (client, err) {
     t.error(err, 'no error')
@@ -179,7 +183,7 @@ test('clients without clientid and clean=true on MQTT 3.1.1 will get a generated
 })
 
 test('client connect error while fetching subscriptions', function (t) {
-  t.plan(1)
+  t.plan(2)
 
   const broker = aedes()
   t.tearDown(broker.close.bind(broker))
@@ -200,6 +204,7 @@ test('client connect error while fetching subscriptions', function (t) {
   })
 
   broker.on('clientError', function (client, err) {
+    t.equal(client.version, 4)
     t.pass('throws error')
   })
 })
@@ -247,7 +252,7 @@ test('client connect clear outgoing', function (t) {
 })
 
 test('clients with zero-byte clientid and clean=true on MQTT 3.1.1 will get a generated clientId', function (t) {
-  t.plan(4)
+  t.plan(5)
 
   const broker = aedes()
   t.tearDown(broker.close.bind(broker))
@@ -266,6 +271,7 @@ test('clients with zero-byte clientid and clean=true on MQTT 3.1.1 will get a ge
     t.equal(packet.cmd, 'connack')
     t.equal(packet.returnCode, 0)
     t.equal(broker.connectedClients, 1)
+    t.equal(s.client.version, 4)
   })
   broker.on('connectionError', function (client, err) {
     t.error(err, 'no error')
@@ -277,7 +283,7 @@ test('clients with zero-byte clientid and clean=true on MQTT 3.1.1 will get a ge
 
 // [MQTT-3.1.3-7]
 test('reject clients with > 23 clientId length in MQTT 3.1.0', function (t) {
-  t.plan(4)
+  t.plan(5)
 
   const broker = aedes()
   t.tearDown(broker.close.bind(broker))
@@ -296,6 +302,7 @@ test('reject clients with > 23 clientId length in MQTT 3.1.0', function (t) {
     t.equal(packet.cmd, 'connack')
     t.equal(packet.returnCode, 2, 'identifier rejected')
     t.equal(broker.connectedClients, 0)
+    t.equal(s.client.version, null)
   })
   broker.on('connectionError', function (client, err) {
     t.equal(err.message, 'identifier rejected')
@@ -303,7 +310,7 @@ test('reject clients with > 23 clientId length in MQTT 3.1.0', function (t) {
 })
 
 test('connect clients with > 23 clientId length using aedes maxClientsIdLength option in MQTT 3.1.0', function (t) {
-  t.plan(3)
+  t.plan(4)
 
   const broker = aedes({ maxClientsIdLength: 26 })
   t.tearDown(broker.close.bind(broker))
@@ -322,6 +329,7 @@ test('connect clients with > 23 clientId length using aedes maxClientsIdLength o
     t.equal(packet.cmd, 'connack')
     t.equal(packet.returnCode, 0)
     t.equal(broker.connectedClients, 1)
+    t.equal(s.client.version, 3)
   })
   broker.on('connectionError', function (client, err) {
     t.error(err, 'no error')
@@ -329,7 +337,7 @@ test('connect clients with > 23 clientId length using aedes maxClientsIdLength o
 })
 
 test('connect with > 23 clientId length in MQTT 3.1.1', function (t) {
-  t.plan(3)
+  t.plan(4)
 
   const broker = aedes()
   t.tearDown(broker.close.bind(broker))
@@ -348,6 +356,7 @@ test('connect with > 23 clientId length in MQTT 3.1.1', function (t) {
     t.equal(packet.cmd, 'connack')
     t.equal(packet.returnCode, 0)
     t.equal(broker.connectedClients, 1)
+    t.equal(s.client.version, 4)
   })
   broker.on('connectionError', function (client, err) {
     t.error(err, 'no error')
@@ -570,7 +579,7 @@ test('Test queue limit', function (t) {
   })
 })
 
-;[['fail with no error msg', 2, null, false], ['succeed with no error msg', 8, null, true], ['fail with error msg', 5, new Error('connection banned'), false], ['succeed with error msg', 5, new Error('connection banned'), true]].forEach(function (ele, idx) {
+;[['fail with no error msg', 3, null, false], ['succeed with no error msg', 9, null, true], ['fail with error msg', 6, new Error('connection banned'), false], ['succeed with error msg', 6, new Error('connection banned'), true]].forEach(function (ele, idx) {
   const title = ele[0]
   const plan = ele[1]
   const err = ele[2]
@@ -582,6 +591,7 @@ test('Test queue limit', function (t) {
       preConnect: function (client, done) {
         t.ok(client.connecting)
         t.notOk(client.connected)
+        t.equal(client.version, null)
         return done(err, ok)
       }
     })
diff --git a/test/types/index.ts b/test/types/index.ts
index fb898cb..86548f1 100644
--- a/test/types/index.ts
+++ b/test/types/index.ts
@@ -19,7 +19,7 @@ const broker = Server({
     }
   },
   authenticate: (client: Client, username: string, password: Buffer, callback) => {
-    if (username === 'test' && password === Buffer.from('test')) {
+    if (username === 'test' && password === Buffer.from('test') && client.version === 4) {
       callback(null, true)
     } else {
       const error = new Error() as AuthenticateError