From 8588c4c00d02604ed92dc91ff3efb61c1bd9a9de Mon Sep 17 00:00:00 2001 From: Pierre Cauchois Date: Thu, 12 Jul 2018 18:24:26 -0700 Subject: [PATCH] Add stop() method to AuthenticationProvider to enable shutting down timers when disconnecting/exiting --- ...ak_authentication_provider_requirements.md | 6 +++ ...as_authentication_provider_requirements.md | 6 ++- device/core/package.json | 8 +-- .../core/src/sak_authentication_provider.ts | 12 +++++ .../core/src/sas_authentication_provider.ts | 8 +++ device/core/test/_client_common_testrun.js | 8 +-- .../test/_sak_authentication_provider_test.js | 51 ++++++++++++++++++- .../test/_sas_authentication_provider_test.js | 16 +++++- device/core/test/http_simulated.js | 4 ++ .../amqp/devdoc/device_amqp_requirements.md | 2 + device/transport/amqp/package.json | 8 +-- device/transport/amqp/src/amqp.ts | 7 ++- device/transport/amqp/test/_amqp_test.js | 10 ++++ .../http/devdoc/http_requirements.md | 2 + device/transport/http/package.json | 8 +-- device/transport/http/src/http.ts | 11 ++-- device/transport/http/test/_http_test.js | 20 ++++++-- .../mqtt/devdoc/mqtt_requirements.md | 2 + device/transport/mqtt/src/mqtt.ts | 7 ++- device/transport/mqtt/test/_mqtt_test.js | 11 ++++ provisioning/device/package.json | 8 +-- .../device/src/polling_state_machine.ts | 19 +++++-- .../test/_polling_state_machine_test.js | 44 +++++++++------- service/package.json | 8 +-- service/src/amqp.ts | 4 +- service/test/_amqp_test.js | 32 ++++++------ 26 files changed, 246 insertions(+), 76 deletions(-) diff --git a/device/core/devdoc/sak_authentication_provider_requirements.md b/device/core/devdoc/sak_authentication_provider_requirements.md index 454682f2e..76f87235d 100644 --- a/device/core/devdoc/sak_authentication_provider_requirements.md +++ b/device/core/devdoc/sak_authentication_provider_requirements.md @@ -46,6 +46,12 @@ sakAuthProvider.on('newTokenAvailable', function (credentials) { **SRS_NODE_SAK_AUTH_PROVIDER_16_008: [** The `fromConnectionString` method shall extract the credentials from the `connectionString` argument and create a new `SharedAccessKeyAuthenticationProvider` that uses these credentials to generate security tokens. **]** +## stop(): void + +**SRS_NODE_SAK_AUTH_PROVIDER_16_012: [** The `stop` method shall clear the token renewal timer if it is running. **]** + +**SRS_NODE_SAK_AUTH_PROVIDER_16_013: [** The `stop` method shall simply return if the token renewal timer is not running. **]** + # Generated Security Token **SRS_NODE_SAK_AUTH_PROVIDER_16_009: [** Every token shall be created with a validity period of `tokenValidTimeInSeconds` if specified when the constructor was called, or 1 hour by default. **]** diff --git a/device/core/devdoc/sas_authentication_provider_requirements.md b/device/core/devdoc/sas_authentication_provider_requirements.md index 1d0223b48..4d5c2a3ad 100644 --- a/device/core/devdoc/sas_authentication_provider_requirements.md +++ b/device/core/devdoc/sas_authentication_provider_requirements.md @@ -41,4 +41,8 @@ sasAuthProvider.on('newTokenAvailable', function (credentials) { **SRS_NODE_SAS_AUTHENTICATION_PROVIDER_16_005: [** The `fromSharedAccessSignature` method shall throw a `ReferenceError` if the `sharedAccessSignature` argument is falsy. **]** -**SRS_NODE_SAS_AUTHENTICATION_PROVIDER_16_006: [** The `fromSharedAccessSignature` shall return a new `SharedAccessSignatureAuthenticationProvider` object initialized with the credentials parsed from the `sharedAccessSignature` argument. **]** \ No newline at end of file +**SRS_NODE_SAS_AUTHENTICATION_PROVIDER_16_006: [** The `fromSharedAccessSignature` shall return a new `SharedAccessSignatureAuthenticationProvider` object initialized with the credentials parsed from the `sharedAccessSignature` argument. **]** + +### stop(): void + +**SRS_NODE_SAS_AUTHENTICATION_PROVIDER_16_007: [** The `stop` method shall simply return since there is no timeout or resources to clear. **]** \ No newline at end of file diff --git a/device/core/package.json b/device/core/package.json index 4b3a90b4e..25719f920 100644 --- a/device/core/package.json +++ b/device/core/package.json @@ -30,10 +30,10 @@ "scripts": { "lint": "tslint --project . -c ../../tslint.json", "build": "tsc", - "unittest-min": "istanbul cover --report none node_modules/mocha/bin/_mocha -- --exit --reporter dot \"test/**/_*_test.js\"", - "alltest-min": "istanbul cover --report none node_modules/mocha/bin/_mocha -- --exit --reporter dot \"test/**/_*_test*.js\"", - "unittest": "istanbul cover node_modules/mocha/bin/_mocha -- --exit --reporter spec \"test/**/_*_test.js\"", - "alltest": "istanbul cover node_modules/mocha/bin/_mocha -- --exit --reporter spec \"test/**/_*_test*.js\"", + "unittest-min": "istanbul cover --report none node_modules/mocha/bin/_mocha -- --reporter dot \"test/**/_*_test.js\"", + "alltest-min": "istanbul cover --report none node_modules/mocha/bin/_mocha -- --reporter dot \"test/**/_*_test*.js\"", + "unittest": "istanbul cover node_modules/mocha/bin/_mocha -- --reporter spec \"test/**/_*_test.js\"", + "alltest": "istanbul cover node_modules/mocha/bin/_mocha -- --reporter spec \"test/**/_*_test*.js\"", "ci": "npm -s run lint && npm -s run build && npm -s run alltest-min && npm -s run check-cover", "test": "npm -s run lint && npm -s run build && npm -s run unittest", "check-cover": "istanbul check-coverage --statements 95 --branches 87 --lines 97 --functions 92" diff --git a/device/core/src/sak_authentication_provider.ts b/device/core/src/sak_authentication_provider.ts index 39e0b87a1..ad8b4f1f2 100644 --- a/device/core/src/sak_authentication_provider.ts +++ b/device/core/src/sak_authentication_provider.ts @@ -66,6 +66,18 @@ export class SharedAccessKeyAuthenticationProvider extends EventEmitter implemen } } + /** + * Stops the timer used to renew to SAS token. + */ + stop(): void { + /*Codes_SRS_NODE_SAK_AUTH_PROVIDER_16_012: [The `stop` method shall clear the token renewal timer if it is running.]*/ + /*Codes_SRS_NODE_SAK_AUTH_PROVIDER_16_013: [The `stop` method shall simply return if the token renewal timer is not running.]*/ + if (this._renewalTimeout) { + clearTimeout(this._renewalTimeout); + this._renewalTimeout = null; + } + } + protected _sign(resourceUri: string, expiry: number, callback: (err: Error, signature?: string) => void): void { callback(null, SharedAccessSignature.create(resourceUri, this._credentials.sharedAccessKeyName, this._credentials.sharedAccessKey, expiry).toString()); } diff --git a/device/core/src/sas_authentication_provider.ts b/device/core/src/sas_authentication_provider.ts index 2987a86f0..90dc90781 100644 --- a/device/core/src/sas_authentication_provider.ts +++ b/device/core/src/sas_authentication_provider.ts @@ -40,6 +40,14 @@ export class SharedAccessSignatureAuthenticationProvider extends EventEmitter im callback(null, this._credentials); } + /** + * does nothing and returns - this is part of the token-based authentication provider API but there are no resources to stop/free here. + */ + stop(): void { + /*Codes_SRS_NODE_SAS_AUTHENTICATION_PROVIDER_16_007: [The `stop` method shall simply return since there is no timeout or resources to clear.]*/ + return; + } + /** * Updates the shared access signature token that transports should use to authenticate. When called, the `SharedAccessSignatureAuthenticationProvider` will emit * a `newTokenAvailable` event that the transports can then use to authenticate with the Azure IoT hub instance. diff --git a/device/core/test/_client_common_testrun.js b/device/core/test/_client_common_testrun.js index 88383f11d..0d88b9c49 100644 --- a/device/core/test/_client_common_testrun.js +++ b/device/core/test/_client_common_testrun.js @@ -37,7 +37,7 @@ function badConfigTests(opName, Client, Transport, requestFn) { client.open(function(err) { if (err) { test(err); - done(); + client.close(done); } else { requestFn(client, function (err, res) { test(err, res); @@ -208,10 +208,12 @@ function batchMessageTests(Client, Transport, registry, testName, requestFn) { requestFn(client, function (err, res) { if (err) { - done(err); + client.close(function () { + done(err); + }); } else { assert.equal(res.constructor.name, 'MessageEnqueued'); - done(); + client.close(done); } }); }); diff --git a/device/core/test/_sak_authentication_provider_test.js b/device/core/test/_sak_authentication_provider_test.js index 0abbb95e6..ba24413c3 100644 --- a/device/core/test/_sak_authentication_provider_test.js +++ b/device/core/test/_sak_authentication_provider_test.js @@ -36,6 +36,7 @@ describe('SharedAccessKeyAuthenticationProvider', function () { assert.notOk(sasObject.skn); assert.isOk(sasObject.sig); assert.isTrue(sasObject.se > Date.now() / 1000); + sakAuthProvider.stop(); testCallback(); }); }); @@ -67,6 +68,7 @@ describe('SharedAccessKeyAuthenticationProvider', function () { assert.isTrue(eventSpy.calledOnce); sakAuthProvider.automaticRenewal = false; testClock.restore(); + sakAuthProvider.stop(); testCallback(); }); }); @@ -90,6 +92,7 @@ describe('SharedAccessKeyAuthenticationProvider', function () { sakAuthProvider.getDeviceCredentials(function (err, creds) { assert.equal(creds.sharedAccessSignature, 'SharedAccessSignature sr=fake.host.name%2Fdevices%2FfakeDeviceId&sig=bYz5R2IFTaejB6pgYOxns2mw6lcuA4VSy8kJbYQp0Sc%3D&se=10'); testClock.restore(); + sakAuthProvider.stop(); testCallback(); }); }); @@ -105,7 +108,8 @@ describe('SharedAccessKeyAuthenticationProvider', function () { var eventSpy = sinon.spy(); sakAuthProvider.on('error', function (err) { assert.strictEqual(err, fakeError); - testCallback(); + sakAuthProvider.stop(); + testCallback(); }); sinon.stub(sakAuthProvider, '_sign').callsArgWith(2, fakeError); @@ -123,7 +127,8 @@ describe('SharedAccessKeyAuthenticationProvider', function () { sinon.stub(sakAuthProvider, '_sign').callsArgWith(2, 'whoops'); sakAuthProvider.getDeviceCredentials(function(err) { assert.equal(err, 'whoops'); - testCallback(); + sakAuthProvider.stop(); + testCallback(); }); }); @@ -139,6 +144,7 @@ describe('SharedAccessKeyAuthenticationProvider', function () { sakAuthProvider._renewToken(function(err, creds) { assert.equal(creds.sharedAccessSignature, 'signature'); }); + sakAuthProvider.stop(); testCallback(); }); @@ -201,9 +207,50 @@ describe('SharedAccessKeyAuthenticationProvider', function () { assert.strictEqual(creds.moduleId, testConfig.credentials.moduleId); assert.strictEqual(creds.host, testConfig.credentials.host); assert.strictEqual(creds.sharedAccessKey, testConfig.credentials.sharedAccessKey); + sakAuthProvider.stop(); testCallback(); }); }); }); }); + + describe('stop', function () { + + /*Tests_SRS_NODE_SAK_AUTH_PROVIDER_16_012: [The `stop` method shall clear the token renewal timer if it is running.]*/ + it('clears the SAS token renewal timeout', function (testCallback) { + this.clock = sinon.useFakeTimers(); + var testClock = this.clock; + var fakeCredentials = { + deviceId: 'fakeDeviceId', + host: 'fake.host.name', + sharedAccessKey: 'fakeKey' + }; + var token; + var sakAuthProvider = new SharedAccessKeyAuthenticationProvider(fakeCredentials, 10, 1); + var eventSpy = sinon.spy(); + sakAuthProvider.on('newTokenAvailable', eventSpy); + /*Tests_SRS_NODE_SAK_AUTH_PROVIDER_16_003: [The `getDeviceCredentials` should call its callback with a `null` first parameter and a `TransportConfig` object as a second parameter, containing the latest valid token it generated.]*/ + sakAuthProvider.getDeviceCredentials(function () { + testClock.tick(11000); // 11 seconds - event should've fired + assert.isTrue(eventSpy.calledOnce); + sakAuthProvider.stop(); + testClock.tick(11000); + assert.isTrue(eventSpy.calledOnce); // if the timer is still running, the event would've fired twice. + testClock.restore(); + testCallback(); + }); + }); + + /*Tests_SRS_NODE_SAK_AUTH_PROVIDER_16_013: [The `stop` method shall simply return if the token renewal timer is not running.]*/ + it('returns and does not crash if the timer is not running', function () { + var sakAuthProvider = new SharedAccessKeyAuthenticationProvider({ + deviceId: 'fakeDeviceId', + host: 'fake.host.name', + sharedAccessKey: 'fakeKey' + }, 10, 1); + assert.doesNotThrow(function () { + sakAuthProvider.stop(); + }); + }); + }); }); \ No newline at end of file diff --git a/device/core/test/_sas_authentication_provider_test.js b/device/core/test/_sas_authentication_provider_test.js index a22e849a9..3d493afef 100644 --- a/device/core/test/_sas_authentication_provider_test.js +++ b/device/core/test/_sas_authentication_provider_test.js @@ -4,8 +4,6 @@ 'use strict'; var assert = require('chai').assert; -var sinon = require('sinon'); -var errors = require('azure-iot-common').errors; var SharedAccessSignature = require('azure-iot-common').SharedAccessSignature; var SharedAccessSignatureAuthenticationProvider = require('../lib/sas_authentication_provider').SharedAccessSignatureAuthenticationProvider; @@ -117,4 +115,18 @@ describe('SharedAccessSignatureAuthenticationProvider', function () { }); }); }); + + describe('#stop', function () { + /*Tests_SRS_NODE_SAS_AUTHENTICATION_PROVIDER_16_007: [The `stop` method shall simply return since there is no timeout or resources to clear.]*/ + it('returns and does not crash if the timer is not running', function () { + var sasAuthProvider = new SharedAccessSignatureAuthenticationProvider({ + host: 'host.name', + deviceId: 'deviceId', + sharedAccessSignature: 'sas' + }, 10, 1); + assert.doesNotThrow(function () { + sasAuthProvider.stop(); + }); + }); + }) }); \ No newline at end of file diff --git a/device/core/test/http_simulated.js b/device/core/test/http_simulated.js index 78b9d2b9d..705c98ba5 100644 --- a/device/core/test/http_simulated.js +++ b/device/core/test/http_simulated.js @@ -32,17 +32,21 @@ function SimulatedHttp(authProvider) { var sig = SharedAccessSignature.parse(config.sharedAccessSignature); if (config.host.indexOf('bad') >= 0) { // bad host + authProvider.stop(); done(new Error('getaddrinfo ENOTFOUND bad')); } else if (config.deviceId.indexOf('bad') >= 0) { // bad policy + authProvider.stop(); done(makeError(404)); } else { var cmpSig = (SharedAccessSignature.create(config.host, config.deviceId, 'bad', sig.se)).toString(); if (config.sharedAccessSignature === cmpSig) { // bad key + authProvider.stop(); done(makeError(401)); } else { + authProvider.stop(); done(null, new results.MessageEnqueued(new Response(204))); } } diff --git a/device/transport/amqp/devdoc/device_amqp_requirements.md b/device/transport/amqp/devdoc/device_amqp_requirements.md index b8e079d36..04b854afb 100644 --- a/device/transport/amqp/devdoc/device_amqp_requirements.md +++ b/device/transport/amqp/devdoc/device_amqp_requirements.md @@ -93,6 +93,8 @@ The `disconnect` method terminates the connection with the Azure IoT Hub instanc **SRS_NODE_DEVICE_AMQP_16_023: [** The `disconnect` method shall forcefully detach all attached links if a connection error is the causing the transport to be disconnected. **]** +**SRS_NODE_DEVICE_AMQP_16_083: [** When the `amqp` client is disconnected and if token-based authentication is used the `stop` method of the `AuthenticationProvider` shall be called. **]** + ### sendEvent(message, done) The `sendEvent` method sends an event to the IoT Hub as the device indicated in the constructor argument. diff --git a/device/transport/amqp/package.json b/device/transport/amqp/package.json index cbb8b9bec..6bee2e331 100644 --- a/device/transport/amqp/package.json +++ b/device/transport/amqp/package.json @@ -30,10 +30,10 @@ "scripts": { "lint": "tslint --project . -c ../../../tslint.json", "build": "tsc", - "unittest-min": "istanbul cover --report none node_modules/mocha/bin/_mocha -- --exit --reporter dot test/_*_test.js", - "alltest-min": "istanbul cover --report none node_modules/mocha/bin/_mocha -- --exit --reporter dot test/_*_test*.js", - "unittest": "istanbul cover node_modules/mocha/bin/_mocha -- --exit --reporter spec test/_*_test.js", - "alltest": "istanbul cover node_modules/mocha/bin/_mocha -- --exit --reporter spec test/_*_test*.js", + "unittest-min": "istanbul cover --report none node_modules/mocha/bin/_mocha -- --reporter dot test/_*_test.js", + "alltest-min": "istanbul cover --report none node_modules/mocha/bin/_mocha -- --reporter dot test/_*_test*.js", + "unittest": "istanbul cover node_modules/mocha/bin/_mocha -- --reporter spec test/_*_test.js", + "alltest": "istanbul cover node_modules/mocha/bin/_mocha -- --reporter spec test/_*_test*.js", "ci": "npm -s run lint && npm -s run build && npm -s run alltest-min && npm -s run check-cover", "test": "npm -s run lint && npm -s run build && npm -s run unittest", "check-cover": "istanbul check-coverage --statements 93 --branches 84 --lines 95 --functions 93" diff --git a/device/transport/amqp/src/amqp.ts b/device/transport/amqp/src/amqp.ts index f585d3cf8..4408eaceb 100644 --- a/device/transport/amqp/src/amqp.ts +++ b/device/transport/amqp/src/amqp.ts @@ -8,7 +8,7 @@ import * as dbg from 'debug'; const debug = dbg('azure-iot-device-amqp:Amqp'); import { EventEmitter } from 'events'; -import { DeviceTransport, MethodMessage, DeviceMethodResponse, TwinProperties, DeviceClientOptions } from 'azure-iot-device'; +import { DeviceTransport, MethodMessage, DeviceMethodResponse, TwinProperties, DeviceClientOptions, SharedAccessKeyAuthenticationProvider } from 'azure-iot-device'; import { getUserAgentString } from 'azure-iot-device'; import { Amqp as BaseAmqpClient, AmqpBaseTransportConfig, translateError, AmqpMessage, SenderLink, ReceiverLink } from 'azure-iot-amqp-base'; import { endpoint, SharedAccessSignature, errors, results, Message, AuthenticationProvider, AuthenticationType, TransportConfig } from 'azure-iot-common'; @@ -147,6 +147,11 @@ export class Amqp extends EventEmitter implements DeviceTransport { states: { disconnected: { _onEnter: (err, callback) => { + /*Codes_SRS_NODE_DEVICE_AMQP_16_083: [When the `amqp` client is disconnected and if token-based authentication is used the `stop` method of the `AuthenticationProvider` shall be called.]*/ + if (this._authenticationProvider.type === AuthenticationType.Token) { + (this._authenticationProvider as SharedAccessKeyAuthenticationProvider).stop(); + } + if (callback) { if (err) { callback(err); diff --git a/device/transport/amqp/test/_amqp_test.js b/device/transport/amqp/test/_amqp_test.js index 5c21f1e53..8dfabb561 100644 --- a/device/transport/amqp/test/_amqp_test.js +++ b/device/transport/amqp/test/_amqp_test.js @@ -66,6 +66,7 @@ describe('Amqp', function () { fakeTokenAuthenticationProvider.type = AuthenticationType.Token; fakeTokenAuthenticationProvider.getDeviceCredentials = sinon.stub().callsArgWith(0, null, configWithSAS); fakeTokenAuthenticationProvider.updateSharedAccessSignature = sinon.stub(); + fakeTokenAuthenticationProvider.stop = sinon.stub(); sinon.spy(fakeTokenAuthenticationProvider, 'on'); fakeX509AuthenticationProvider = { @@ -703,6 +704,15 @@ describe('Amqp', function () { }); }); }); + + /*Tests_SRS_NODE_DEVICE_AMQP_16_083: [When the `amqp` client is disconnected and if token-based authentication is used the `stop` method of the `AuthenticationProvider` shall be called.]*/ + it('calls stop on the authentication provider if using token authentication', function (testCallback) { + transport.connect(function () {}); + transport.disconnect(function () { + assert.isTrue(fakeTokenAuthenticationProvider.stop.calledTwice); // once when instantiated, once when disconnected + testCallback(); + }); + }); }); describe('#updateSharedAccessSignature', function () { diff --git a/device/transport/http/devdoc/http_requirements.md b/device/transport/http/devdoc/http_requirements.md index feea72e6c..40fc3018d 100644 --- a/device/transport/http/devdoc/http_requirements.md +++ b/device/transport/http/devdoc/http_requirements.md @@ -62,6 +62,8 @@ or: **SRS_NODE_DEVICE_HTTP_16_031: [** The `disconnect` method shall call its callback with a `null` first argument and a `results.Disconnected` second argument after successfully disabling the C2D receiver (if necessary). **]** +**SRS_NODE_DEVICE_HTTP_16_039: [** The `disconnect` method shall call the `stop` method on the `AuthenticationProvider` object if the type of authentication used is "token". **]** + ### sendEvent(message, done) The `sendEvent` method sends an event to an IoT hub on behalf of the device indicated in the constructor argument. diff --git a/device/transport/http/package.json b/device/transport/http/package.json index 989a7dbff..a9528f41d 100644 --- a/device/transport/http/package.json +++ b/device/transport/http/package.json @@ -26,10 +26,10 @@ "scripts": { "lint": "tslint --project . -c ../../../tslint.json", "build": "tsc", - "unittest-min": "istanbul cover --report none node_modules/mocha/bin/_mocha -- --exit --reporter dot test/_*_test.js", - "alltest-min": "istanbul cover --report none node_modules/mocha/bin/_mocha -- --exit --reporter dot test/_*_test*.js", - "unittest": "istanbul cover node_modules/mocha/bin/_mocha -- --exit --reporter spec test/_*_test.js", - "alltest": "istanbul cover node_modules/mocha/bin/_mocha -- --exit --reporter spec test/_*_test*.js", + "unittest-min": "istanbul cover --report none node_modules/mocha/bin/_mocha -- --reporter dot test/_*_test.js", + "alltest-min": "istanbul cover --report none node_modules/mocha/bin/_mocha -- --reporter dot test/_*_test*.js", + "unittest": "istanbul cover node_modules/mocha/bin/_mocha -- --reporter spec test/_*_test.js", + "alltest": "istanbul cover node_modules/mocha/bin/_mocha -- --reporter spec test/_*_test*.js", "ci": "npm -s run lint && npm -s run build && npm -s run alltest-min && npm -s run check-cover", "test": "npm -s run lint && npm -s run build && npm -s run unittest", "check-cover": "istanbul check-coverage --statements 92 --branches 82 --lines 93 --functions 91" diff --git a/device/transport/http/src/http.ts b/device/transport/http/src/http.ts index b191f21ff..9140e0835 100644 --- a/device/transport/http/src/http.ts +++ b/device/transport/http/src/http.ts @@ -12,7 +12,7 @@ import { Http as Base } from 'azure-iot-http-base'; import { endpoint, errors, results, Message, AuthenticationProvider, AuthenticationType, TransportConfig, encodeUriComponentStrict } from 'azure-iot-common'; import { translateError } from './http_errors.js'; import { IncomingMessage } from 'http'; -import { DeviceTransport, MethodMessage, DeviceMethodResponse, TwinProperties } from 'azure-iot-device'; +import { DeviceTransport, MethodMessage, DeviceMethodResponse, TwinProperties, SharedAccessKeyAuthenticationProvider } from 'azure-iot-device'; import { X509AuthenticationProvider, SharedAccessSignatureAuthenticationProvider } from 'azure-iot-device'; import { DeviceClientOptions, HttpReceiverOptions } from 'azure-iot-device'; import { getUserAgentString } from 'azure-iot-device'; @@ -99,6 +99,11 @@ export class Http extends EventEmitter implements DeviceTransport { * @private */ disconnect(callback: (err?: Error, result?: results.Disconnected) => void): void { + /*Codes_SRS_NODE_DEVICE_HTTP_16_039: [The `disconnect` method shall call the `stop` method on the `AuthenticationProvider` object if the type of authentication used is "token".]*/ + if (this._authenticationProvider.type === AuthenticationType.Token) { + (this._authenticationProvider as SharedAccessKeyAuthenticationProvider).stop(); + } + if (this._receiverStarted) { /*Codes_SRS_NODE_DEVICE_HTTP_16_029: [The `disconnect` method shall disable the C2D message receiver if it is running. ]*/ this.disableC2D((err) => { @@ -111,8 +116,8 @@ export class Http extends EventEmitter implements DeviceTransport { } }); } else { - /*Codes_SRS_NODE_DEVICE_HTTP_16_031: [The `disconnect` method shall call its callback with a `null` first argument and a `results.Disconnected` second argument after successfully disabling the C2D receiver (if necessary). ]*/ - callback(null, new results.Disconnected()); + /*Codes_SRS_NODE_DEVICE_HTTP_16_031: [The `disconnect` method shall call its callback with a `null` first argument and a `results.Disconnected` second argument after successfully disabling the C2D receiver (if necessary). ]*/ + callback(null, new results.Disconnected()); } } diff --git a/device/transport/http/test/_http_test.js b/device/transport/http/test/_http_test.js index c8476f233..9cb1dd73c 100644 --- a/device/transport/http/test/_http_test.js +++ b/device/transport/http/test/_http_test.js @@ -10,6 +10,7 @@ var Message = require('azure-iot-common').Message; var results = require('azure-iot-common').results; var ArgumentError = require('azure-iot-common').errors.ArgumentError; var NotImplementedError = require('azure-iot-common').errors.NotImplementedError; +var AuthenticationType = require('azure-iot-common').AuthenticationType; var Http = require('../lib/http.js').Http; var FakeHttp = function () { }; @@ -44,10 +45,12 @@ describe('Http', function () { beforeEach(function () { fakeAuthenticationProvider = { + type: AuthenticationType.Token, getDeviceCredentials: sinon.stub().callsFake(function (callback) { callback(null, { host: 'hub.host.name', deviceId: 'deviceId', sharedAccessSignature: 'sas.key' }); }), - updateSharedAccessSignature: sinon.stub() + updateSharedAccessSignature: sinon.stub(), + stop: sinon.stub() }; transport = new Http(fakeAuthenticationProvider); }); @@ -121,6 +124,16 @@ describe('Http', function () { }); }); }); + + /*Tests_SRS_NODE_DEVICE_HTTP_16_039: [The `disconnect` method shall call the `stop` method on the `AuthenticationProvider` object if the type of authentication used is "token".]*/ + it('calls stop on the authentication provider if using token authentication', function (testCallback) { + var http = new Http(fakeAuthenticationProvider); + http.connect(function () {}); + http.disconnect(function () { + assert.isTrue(fakeAuthenticationProvider.stop.calledOnce); + testCallback(); + }); + }); }); describe('#sendEvent', function() { @@ -365,7 +378,8 @@ describe('HttpReceiver', function () { getDeviceCredentials: sinon.stub().callsFake(function (callback) { callback(null, { host: 'hub.host.name', deviceId: 'deviceId', sharedAccessSignature: 'sas.key' }); }), - updateSharedAccessSignature: sinon.stub() + updateSharedAccessSignature: sinon.stub(), + stop: sinon.stub() }; }); @@ -679,7 +693,7 @@ describe('HttpReceiver', function () { http.setOptions({ interval: 1, at: null, cron: null, drain: true }); assert.isTrue(http.disableC2D.calledOnce); assert.isTrue(http.enableC2D.calledOnce); - testCallback(); + http.disconnect(testCallback); }); }); }); diff --git a/device/transport/mqtt/devdoc/mqtt_requirements.md b/device/transport/mqtt/devdoc/mqtt_requirements.md index ccf48c04f..a53368067 100644 --- a/device/transport/mqtt/devdoc/mqtt_requirements.md +++ b/device/transport/mqtt/devdoc/mqtt_requirements.md @@ -76,6 +76,8 @@ The `disconnect` method should close the connection to the IoT Hub instance. **SRS_NODE_DEVICE_MQTT_16_022: [** The `disconnect` method shall call its callback with a `null` error parameter and a `results.Disconnected` response if `MqttBase` successfully disconnects if not disconnected already. **]** +**SRS_NODE_DEVICE_MQTT_16_085: [** Once the MQTT transport is disconnected and if it is using a token authentication provider, the `stop` method of the `AuthenticationProvider` object shall be called to stop any running timer. **]** + ### sendEvent(message) The `sendEvent` method sends an event to an IoT hub on behalf of the device indicated in the constructor argument. diff --git a/device/transport/mqtt/src/mqtt.ts b/device/transport/mqtt/src/mqtt.ts index 4b434107e..553f8fabe 100644 --- a/device/transport/mqtt/src/mqtt.ts +++ b/device/transport/mqtt/src/mqtt.ts @@ -7,7 +7,7 @@ import * as URL from 'url'; import * as machina from 'machina'; import { endpoint, results, errors, Message, AuthenticationProvider, AuthenticationType, TransportConfig } from 'azure-iot-common'; -import { MethodMessage, DeviceMethodResponse, DeviceTransport, DeviceClientOptions, TwinProperties } from 'azure-iot-device'; +import { MethodMessage, DeviceMethodResponse, DeviceTransport, DeviceClientOptions, TwinProperties, SharedAccessKeyAuthenticationProvider } from 'azure-iot-device'; import { X509AuthenticationProvider, SharedAccessSignatureAuthenticationProvider } from 'azure-iot-device'; import { getUserAgentString } from 'azure-iot-device'; import { EventEmitter } from 'events'; @@ -93,6 +93,11 @@ export class Mqtt extends EventEmitter implements DeviceTransport { states: { disconnected: { _onEnter: (disconnectedCallback, err, result) => { + /*Codes_SRS_NODE_DEVICE_MQTT_16_085: [Once the MQTT transport is disconnected and if it is using a token authentication provider, the `stop` method of the `AuthenticationProvider` object shall be called to stop any running timer.]*/ + if (this._authenticationProvider.type === AuthenticationType.Token) { + (this._authenticationProvider as SharedAccessKeyAuthenticationProvider).stop(); + } + if (disconnectedCallback) { if (err) { /*Codes_SRS_NODE_DEVICE_MQTT_16_019: [The `connect` method shall calls its callback with an `Error` that has been translated from the `MqttBase` error using the `translateError` method if it fails to establish a connection.]*/ diff --git a/device/transport/mqtt/test/_mqtt_test.js b/device/transport/mqtt/test/_mqtt_test.js index 08d57932b..cece2bd2e 100644 --- a/device/transport/mqtt/test/_mqtt_test.js +++ b/device/transport/mqtt/test/_mqtt_test.js @@ -29,6 +29,7 @@ describe('Mqtt', function () { fakeAuthenticationProvider.type = AuthenticationType.Token; fakeAuthenticationProvider.getDeviceCredentials = sinon.stub().callsArgWith(0, null, fakeConfig); fakeAuthenticationProvider.updateSharedAccessSignature = sinon.stub(); + fakeAuthenticationProvider.stop = sinon.stub(); sinon.spy(fakeAuthenticationProvider, 'on'); fakeMqttBase = new EventEmitter(); @@ -706,6 +707,16 @@ describe('Mqtt', function () { testCallback(); }); }); + + /*Tests_SRS_NODE_DEVICE_MQTT_16_085: [Once the MQTT transport is disconnected and if it is using a token authentication provider, the `stop` method of the `AuthenticationProvider` object shall be called to stop any running timer.]*/ + it('calls stop on the authentication provider if using token authentication', function (testCallback) { + var mqtt = new Mqtt(fakeAuthenticationProvider, fakeMqttBase); + mqtt.connect(function () {}); + mqtt.disconnect(function () { + assert.isTrue(fakeAuthenticationProvider.stop.calledTwice); // once when instantiated, once when disconnected + testCallback(); + }); + }); }); describe('#updateSharedAccessSignature', function () { diff --git a/provisioning/device/package.json b/provisioning/device/package.json index 9cc19cd2a..4e97b6db6 100644 --- a/provisioning/device/package.json +++ b/provisioning/device/package.json @@ -23,10 +23,10 @@ "scripts": { "lint": "tslint --project . -c ../../tslint.json", "build": "tsc", - "unittest-min": "istanbul cover node_modules/mocha/bin/_mocha -- --exit --reporter dot test/_*_test.js", - "alltest-min": "istanbul cover node_modules/mocha/bin/_mocha -- --exit --reporter dot test/_*_test*.js", - "unittest": "istanbul cover node_modules/mocha/bin/_mocha -- --exit --reporter spec test/_*_test.js", - "alltest": "istanbul cover node_modules/mocha/bin/_mocha -- --exit --reporter spec test/_*_test*.js", + "unittest-min": "istanbul cover node_modules/mocha/bin/_mocha -- --reporter dot test/_*_test.js", + "alltest-min": "istanbul cover node_modules/mocha/bin/_mocha -- --reporter dot test/_*_test*.js", + "unittest": "istanbul cover node_modules/mocha/bin/_mocha -- --reporter spec test/_*_test.js", + "alltest": "istanbul cover node_modules/mocha/bin/_mocha -- --reporter spec test/_*_test*.js", "ci": "npm -s run lint && npm -s run build && npm -s run alltest-min && npm -s run check-cover", "test": "npm -s run lint && npm -s run build && npm -s run unittest", "check-cover": "istanbul check-coverage --statements 84 --branches 63 --functions 73 --lines 85" diff --git a/provisioning/device/src/polling_state_machine.ts b/provisioning/device/src/polling_state_machine.ts index ba4e3e648..c28eca785 100644 --- a/provisioning/device/src/polling_state_machine.ts +++ b/provisioning/device/src/polling_state_machine.ts @@ -17,6 +17,7 @@ const debug = dbg('azure-iot-provisioning-device:PollingStateMachine'); export class PollingStateMachine extends EventEmitter { private _fsm: machina.Fsm; private _pollingTimer: any; + private _queryTimer: any; private _transport: PollingTransport; private _currentOperationCallback: any; @@ -51,7 +52,7 @@ export class PollingStateMachine extends EventEmitter { }, sendingRegistrationRequest: { _onEnter: (request, callback) => { - let timeoutTimer = setTimeout(() => { + this._queryTimer = setTimeout(() => { /* Codes_SRS_NODE_PROVISIONING_TRANSPORT_STATE_MACHINE_18_036: [ If `PollingTransport.registrationRequest` does not call its callback within `ProvisioningDeviceConstants.defaultTimeoutInterval` ms, register shall with with a `TimeoutError` error. ] */ if (this._currentOperationCallback === callback) { debug('timeout while sending request'); @@ -62,7 +63,7 @@ export class PollingStateMachine extends EventEmitter { /* Codes_SRS_NODE_PROVISIONING_TRANSPORT_STATE_MACHINE_18_012: [ `register` shall call `PollingTransport.registrationRequest`. ] */ this._currentOperationCallback = callback; this._transport.registrationRequest(request, (err, result, response, pollingInterval) => { - clearTimeout(timeoutTimer); + clearTimeout(this._queryTimer); // Check if the operation is still pending before transitioning. We might be in a different state now and we don't want to mess that up. if (this._currentOperationCallback === callback) { this._fsm.transition('responseReceived', err, request, result, response, pollingInterval, callback); @@ -165,7 +166,7 @@ export class PollingStateMachine extends EventEmitter { polling: { _onEnter: (request, operationId, pollingInterval, callback) => { /* Codes_SRS_NODE_PROVISIONING_TRANSPORT_STATE_MACHINE_18_037: [ If `PollingTransport.queryOperationStatus` does not call its callback within `ProvisioningDeviceConstants.defaultTimeoutInterval` ms, register shall with with a `TimeoutError` error. ] */ - let timeoutTimer: any = setTimeout(() => { + this._queryTimer = setTimeout(() => { debug('timeout while query'); if (this._currentOperationCallback === callback) { /* tslint:disable:no-empty */ @@ -174,7 +175,7 @@ export class PollingStateMachine extends EventEmitter { }, ProvisioningDeviceConstants.defaultTimeoutInterval); /* Codes_SRS_NODE_PROVISIONING_TRANSPORT_STATE_MACHINE_18_018: [ When the polling interval elapses, `register` shall call `PollingTransport.queryOperationStatus`. ] */ this._transport.queryOperationStatus(request, operationId, (err, result, response, pollingInterval) => { - clearTimeout(timeoutTimer); + clearTimeout(this._queryTimer); // Check if the operation is still pending before transitioning. We might be in a different state now and we don't want to mess that up. if (this._currentOperationCallback === callback) { this._fsm.transition('responseReceived', err, request, result, response, pollingInterval, callback); @@ -212,6 +213,16 @@ export class PollingStateMachine extends EventEmitter { }, disconnecting: { _onEnter: (callback) => { + if (this._pollingTimer) { + debug('cancelling polling timer'); + clearTimeout(this._pollingTimer); + } + + if (this._queryTimer) { + debug('cancelling query timer'); + clearTimeout(this._queryTimer); + } + this._transport.disconnect((err) => { this._fsm.transition('disconnected', err, null, null, callback); }); diff --git a/provisioning/device/test/_polling_state_machine_test.js b/provisioning/device/test/_polling_state_machine_test.js index 1ff077d38..f199088c2 100644 --- a/provisioning/device/test/_polling_state_machine_test.js +++ b/provisioning/device/test/_polling_state_machine_test.js @@ -42,7 +42,7 @@ var operationStatusReturnsAssigningThenAssigned = function () { }); }; -describe('state machine', function () { +describe('polling state machine', function () { this.timeout(1000); var makeNewMachine = function () { @@ -79,7 +79,7 @@ describe('state machine', function () { describe('calls registrationRequest', function () { /* Tests_SRS_NODE_PROVISIONING_TRANSPORT_STATE_MACHINE_18_013: [ If `PollingTransport.registrationRequest` fails, `register` shall fail. ] */ - it ('and returns failure if it fails', function (testCallback) { + it('and returns failure if it fails', function (testCallback) { machine._transport.registrationRequest = registrationRequestReturnsFailure(); callRegisterWithDefaultArgs(function (err) { assert.instanceOf(err, Error); @@ -98,19 +98,20 @@ describe('state machine', function () { }); /* Tests_SRS_NODE_PROVISIONING_TRANSPORT_STATE_MACHINE_18_015: [ If `PollingTransport.registrationRequest` succeeds with status==Assigning, it shall emit an 'operationStatus' event and begin polling for operation status requests. ] */ - it ('and starts polling if it succeeds with status===\'Assigning\'', function (testCallback) { + it('and starts polling if it succeeds with status===\'Assigning\'', function (testCallback) { machine._transport.registrationRequest = registrationRequestReturnsAssigning(); machine._transport.queryOperationStatus = waitingForNetworkIo(); callRegisterWithDefaultArgs(function () {}); setTimeout(function () { assert(machine._transport.registrationRequest.calledOnce); assert(machine._transport.queryOperationStatus.calledOnce); + machine.disconnect(); testCallback(); }, 2); }); /* Tests_SRS_NODE_PROVISIONING_TRANSPORT_STATE_MACHINE_18_016: [ If `PollingTransport.registrationRequest` succeeds with an unknown status, `register` shall fail with a `SyntaxError` and pass the response body and the protocol-specific result to the `callback`. ] */ - it ('and returns failure if it succeeds with some other status', function (testCallback) { + it('and returns failure if it succeeds with some other status', function (testCallback) { machine._transport.registrationRequest = registrationRequestReturnsBadResponse(); callRegisterWithDefaultArgs(function (err) { assert.instanceOf(err, SyntaxError); @@ -120,7 +121,7 @@ describe('state machine', function () { }); /* Tests_SRS_NODE_PROVISIONING_TRANSPORT_STATE_MACHINE_18_028: [ If `PollingTransport.registrationRequest` succeeds with status==Failed, it shall fail with a `DeviceRegistrationFailedError` error ] */ - it ('and returns failure if status==Failed', function (testCallback) { + it('and returns failure if status==Failed', function (testCallback) { machine._transport.registrationRequest = registrationRequestReturnsFailed(); callRegisterWithDefaultArgs(function (err) { assert.instanceOf(err, errors.DeviceRegistrationFailedError); @@ -130,7 +131,7 @@ describe('state machine', function () { /* Tests_SRS_NODE_PROVISIONING_TRANSPORT_STATE_MACHINE_18_014: [ If `PollingTransport.registrationRequest` succeeds with status==Assigned, it shall emit an 'operationStatus' event and call `callback` with null, the response body, and the protocol-specific result. ] */ - it ('and fires an operationStatus event if it succeeds', function (testCallback) { + it('and fires an operationStatus event if it succeeds', function (testCallback) { machine.on('operationStatus', function (body) { assert.strictEqual(body.status, 'Assigned'); testCallback(); @@ -210,21 +211,23 @@ describe('state machine', function () { }); /* Tests_SRS_NODE_PROVISIONING_TRANSPORT_STATE_MACHINE_18_024: [ If `register` is called while a different request is in progress, it shall fail with an `InvalidOperationError`. ] */ - it ('fails if called while sending the first request', function (testCallback) { + it('fails if called while sending the first request', function (testCallback) { machine._transport.registrationRequest = waitingForNetworkIo(); callRegisterWithDefaultArgs(function() {}); setTimeout(function() { callRegisterWithDefaultArgs(function(err) { assert.strictEqual(err.constructor.name, 'InvalidOperationError'); - machine.cancel(function() {}); - testCallback(); + machine.cancel(function() { + machine.disconnect(); + testCallback(); + }); }); - },10); + }, 10); }); /* Tests_SRS_NODE_PROVISIONING_TRANSPORT_STATE_MACHINE_18_024: [ If `register` is called while a different request is in progress, it shall fail with an `InvalidOperationError`. ] */ - it ('fails if called while waiting to poll', function (testCallback) { + it('fails if called while waiting to poll', function (testCallback) { machine._transport.registrationRequest = registrationRequestReturnsAssigning(1000); callRegisterWithDefaultArgs(function() {}); @@ -238,7 +241,7 @@ describe('state machine', function () { }); /* Tests_SRS_NODE_PROVISIONING_TRANSPORT_STATE_MACHINE_18_024: [ If `register` is called while a different request is in progress, it shall fail with an `InvalidOperationError`. ] */ - it ('fails if called while sending an operation status request', function (testCallback) { + it('fails if called while sending an operation status request', function (testCallback) { machine._transport.registrationRequest = registrationRequestReturnsAssigning(1); machine._transport.queryOperationStatus = waitingForNetworkIo(); callRegisterWithDefaultArgs(function() {}); @@ -246,8 +249,10 @@ describe('state machine', function () { setTimeout(function() { callRegisterWithDefaultArgs(function(err) { assert.strictEqual(err.constructor.name, 'InvalidOperationError'); - machine.cancel(function() {}); - testCallback(); + machine.cancel(function() { + machine.disconnect(); + testCallback(); + }); }); },2); }); @@ -256,7 +261,7 @@ describe('state machine', function () { describe('cancel function', function () { /* Tests_SRS_NODE_PROVISIONING_TRANSPORT_STATE_MACHINE_18_025: [ If `cancel` is called while disconnected, it shall immediately call its `callback`. ] */ - it ('does nothing if called while disconnected', function (testCallback) { + it('does nothing if called while disconnected', function (testCallback) { machine.cancel(function(err) { assert.oneOf(err, [null, undefined]); assert(machine); @@ -266,7 +271,7 @@ describe('state machine', function () { }); /* Tests_SRS_NODE_PROVISIONING_TRANSPORT_STATE_MACHINE_18_030: [ If `cancel` is called while the transport is connected but idle, it shall immediately call its `callback`. ] */ - it ('does nothing if called while idle', function (testCallback) { + it('does nothing if called while idle', function (testCallback) { callRegisterWithDefaultArgs(function(err) { assert.oneOf(err, [null, undefined]); testCallback(); @@ -276,7 +281,7 @@ describe('state machine', function () { describe('calls cancel', function () { /* Tests_SRS_NODE_PROVISIONING_TRANSPORT_STATE_MACHINE_18_027: [ If a registration is in progress, `cancel` shall cause that registration to fail with an `OperationCancelledError`. ] */ - it ('and causes register to fail if called while sending the first request', function (testCallback) { + it('and causes register to fail if called while sending the first request', function (testCallback) { var registrationErr; var registrationCallback; machine._transport.registrationRequest = sinon.spy(function (request, callback) { @@ -299,7 +304,7 @@ describe('state machine', function () { }); /* Tests_SRS_NODE_PROVISIONING_TRANSPORT_STATE_MACHINE_18_027: [ If a registration is in progress, `cancel` shall cause that registration to fail with an `OperationCancelledError`. ] */ - it ('and causes register to fail if called while waiting to poll', function (testCallback) { + it('and causes register to fail if called while waiting to poll', function (testCallback) { var registrationErr; machine._transport.registrationRequest = registrationRequestReturnsAssigning(1000); callRegisterWithDefaultArgs(function(err) { @@ -320,7 +325,7 @@ describe('state machine', function () { }); /* Tests_SRS_NODE_PROVISIONING_TRANSPORT_STATE_MACHINE_18_027: [ If a registration is in progress, `cancel` shall cause that registration to fail with an `OperationCancelledError`. ] */ - it ('and causes register to fail if called while sending an operation status request', function (testCallback) { + it('and causes register to fail if called while sending an operation status request', function (testCallback) { var registrationErr; machine._transport.registrationRequest = registrationRequestReturnsAssigning(); machine._transport.queryOperationStatus = sinon.spy(function() { @@ -328,6 +333,7 @@ describe('state machine', function () { setTimeout(function() { assert(!!registrationErr); assert(machine._transport.cancel.calledOnce); + machine.disconnect(); testCallback(); }, 2); }); diff --git a/service/package.json b/service/package.json index 269fa98dd..9ace02d50 100644 --- a/service/package.json +++ b/service/package.json @@ -31,10 +31,10 @@ "lint": "tslint --exclude ./samples --project . -c ../tslint.json", "typings": "typings install", "build": "npm run typings && tsc", - "unittest-min": "istanbul cover --report none node_modules/mocha/bin/_mocha -- --exit --reporter dot test/_*_test.js", - "alltest-min": "istanbul cover --report none node_modules/mocha/bin/_mocha -- --exit --reporter dot test/_*_test*.js", - "unittest": "istanbul cover node_modules/mocha/bin/_mocha -- --exit --reporter spec test/_*_test.js", - "alltest": "istanbul cover node_modules/mocha/bin/_mocha -- --exit --reporter spec test/_*_test*.js", + "unittest-min": "istanbul cover --report none node_modules/mocha/bin/_mocha -- --reporter dot test/_*_test.js", + "alltest-min": "istanbul cover --report none node_modules/mocha/bin/_mocha -- --reporter dot test/_*_test*.js", + "unittest": "istanbul cover node_modules/mocha/bin/_mocha -- --reporter spec test/_*_test.js", + "alltest": "istanbul cover node_modules/mocha/bin/_mocha -- --reporter spec test/_*_test*.js", "ci": "npm -s run lint && npm -s run build && npm -s run alltest-min && npm -s run check-cover", "test": "npm -s run lint && npm -s run build && npm -s run unittest", "check-cover": "istanbul check-coverage --statements 97 --branches 93 --lines 98 --functions 93", diff --git a/service/src/amqp.ts b/service/src/amqp.ts index b39d5bc4d..cd21d9643 100644 --- a/service/src/amqp.ts +++ b/service/src/amqp.ts @@ -195,7 +195,7 @@ export class Amqp extends EventEmitter implements Client.Transport { if (err) { debug('error trying to initialize CBS: ' + err.toString()); /*Codes_SRS_NODE_IOTHUB_SERVICE_AMQP_06_002: [If `initializeCBS` is not successful then the client will remain disconnected and the callback, if provided, will be invoked with an error object.]*/ - this._fsm.transition('disconnecting', callback); + this._fsm.transition('disconnecting', err, callback); } else { debug('CBS initialized'); /*Codes_SRS_NODE_IOTHUB_SERVICE_AMQP_06_003: [If `initializeCBS` is successful, `putToken` shall be invoked with the first parameter audience, created from the sr of the sas signature, the next parameter of the actual sas, and a callback.]*/ @@ -205,7 +205,7 @@ export class Amqp extends EventEmitter implements Client.Transport { this._amqp.putToken(audience, sasToken, (err) => { if (err) { /*Codes_SRS_NODE_IOTHUB_SERVICE_AMQP_06_004: [** If `putToken` is not successful then the client will remain disconnected and the callback, if provided, will be invoked with an error object.]*/ - this._fsm.transition('disconnecting', err); + this._fsm.transition('disconnecting', err, callback); } else { this._fsm.transition('authenticated', applicationSuppliedSas, callback); } diff --git a/service/test/_amqp_test.js b/service/test/_amqp_test.js index e59887ecb..3a56ff5fd 100644 --- a/service/test/_amqp_test.js +++ b/service/test/_amqp_test.js @@ -114,8 +114,13 @@ describe('Amqp', function() { describe('#connect', function() { /*Tests_SRS_NODE_IOTHUB_SERVICE_AMQP_16_019: [The `connect` method shall call the `connect` method of the base AMQP transport and translate its result to the caller into a transport-agnostic object.]*/ it('calls the base transport connect method', function(done) { - var amqp = new Amqp(fakeConfig, fakeAmqpBase); - amqp.connect(done); + var amqp = new Amqp(sasConfig, fakeAmqpBase); + amqp.connect(function () { + assert.isTrue(fakeAmqpBase.connect.calledOnce); + amqp.disconnect(function () { + done(); + }); + }); }); /*Tests_SRS_NODE_IOTHUB_SERVICE_AMQP_16_017: [All asynchronous instance methods shall call the `done` callback with a single parameter that is derived from the standard Javascript `Error` object if the operation failed.]*/ @@ -125,22 +130,15 @@ describe('Amqp', function() { var amqp = new Amqp(fakeConfig, fakeAmqpBase); amqp.connect(function (err) { assert.instanceOf(err, Error); - done(); - }); - }); - - /*Tests_SRS_NODE_IOTHUB_SERVICE_AMQP_16_019: [The `connect` method shall call the `connect` method of the base AMQP transport and translate its result to the caller into a transport-agnostic object.]*/ - it('calls the base transport connect method with renewable sas config', function(testCallback) { - var amqp = new Amqp(sasConfig, fakeAmqpBase); - amqp.connect(function () { - assert.isTrue(fakeAmqpBase.connect.calledOnce); - testCallback(); + amqp.disconnect(function () { + done(); + }); }); }); /*Tests_SRS_NODE_IOTHUB_SERVICE_AMQP_06_001: [`initializeCBS` shall be invoked.]*/ /*Tests_SRS_NODE_IOTHUB_SERVICE_AMQP_06_002: [If `initializeCBS` is not successful then the client will remain disconnected and the callback, if provided, will be invoked with an error object.]*/ - it('Invokes initializeCBS - initialize fails and disconnects', function () { + it('Invokes initializeCBS - initialize fails and disconnects', function (testCallback) { var testError = new errors.InternalServerError('fake error'); fakeAmqpBase.initializeCBS = sinon.stub().callsArgWith(0, testError); var transport = new Amqp(sasConfig, fakeAmqpBase); @@ -148,12 +146,13 @@ describe('Amqp', function() { assert(fakeAmqpBase.initializeCBS.calledOnce); assert(fakeAmqpBase.disconnect.calledOnce); assert.instanceOf(err, Error); + testCallback(); }); }); /*Tests_SRS_NODE_IOTHUB_SERVICE_AMQP_06_003: [If `initializeCBS` is successful, `putToken` shall be invoked with the first parameter audience, created from the sr of the sas signature, the next parameter of the actual sas, and a callback.]*/ /*Tests_SRS_NODE_IOTHUB_SERVICE_AMQP_06_004: [If `putToken` is not successful then the client will remain disconnected and the callback, if provided, will be invoked with an error object.]*/ - it('Invokes putToken - puttoken fails and disconnects', function () { + it('Invokes putToken - puttoken fails and disconnects', function (testCallback) { var testError = new errors.NotConnectedError('fake error'); fakeAmqpBase.putToken = sinon.stub().callsArgWith(2, testError); var transport = new Amqp(sasConfig, fakeAmqpBase); @@ -162,6 +161,7 @@ describe('Amqp', function() { assert(fakeAmqpBase.putToken.calledWith('uri',sasConfig.sharedAccessSignature.toString())); assert(fakeAmqpBase.disconnect.calledOnce); assert.instanceOf(err, Error); + testCallback(); }); }); @@ -175,7 +175,9 @@ describe('Amqp', function() { assert.isTrue(fakeAmqpBase.connect.calledOnce); assert.isTrue(fakeAmqpBase.initializeCBS.calledOnce); assert.isTrue(fakeAmqpBase.putToken.calledOnce); - testCallback(); + transport.disconnect(function () { + testCallback(); + }); }); }); });