Code review feedback
This commit is contained in:
Родитель
91b820c302
Коммит
148d0d9129
|
@ -30,7 +30,7 @@
|
|||
"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 87 --branches 80 --functions 76 --lines 87"
|
||||
"check-cover": "istanbul check-coverage --statements 86 --branches 80 --functions 76 --lines 86"
|
||||
},
|
||||
"engines": {
|
||||
"node": ">= 0.10"
|
||||
|
|
|
@ -152,9 +152,8 @@ export class Http {
|
|||
/**
|
||||
* @private
|
||||
*/
|
||||
setOptions(options: any, callback: (err?: Error) => void): void {
|
||||
setOptions(options: any): void {
|
||||
this._options = options;
|
||||
if (callback) callback();
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -173,9 +173,9 @@ export class RestApiClient {
|
|||
/**
|
||||
* @private
|
||||
*/
|
||||
setOptions(options: any, callback?: (err?: Error) => void): void {
|
||||
setOptions(options: any): void {
|
||||
/*Codes_SRS_NODE_IOTHUB_REST_API_CLIENT_18_003: [ `setOptions` shall call `this._http.setOptions` passing the same parameters ]*/
|
||||
this._http.setOptions(options, callback);
|
||||
this._http.setOptions(options);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -47,14 +47,12 @@ describe('Http', function() {
|
|||
});
|
||||
sinon.stub(https,'request').returns(new EventEmitter());
|
||||
var http = new Http();
|
||||
http.setOptions(fakeOptions, function(err) {
|
||||
assert(!err);
|
||||
http.buildRequest('GET', fakePath, fakeHeaders, fakeHost, function() {});
|
||||
assert(https.request.called);
|
||||
var httpOptions = https.request.firstCall.args[0];
|
||||
assert.strictEqual(httpOptions.agent, fakeOptions.http.agent);
|
||||
callback();
|
||||
});
|
||||
http.setOptions(fakeOptions);
|
||||
http.buildRequest('GET', fakePath, fakeHeaders, fakeHost, function() {});
|
||||
assert(https.request.called);
|
||||
var httpOptions = https.request.firstCall.args[0];
|
||||
assert.strictEqual(httpOptions.agent, fakeOptions.http.agent);
|
||||
callback();
|
||||
});
|
||||
|
||||
});
|
||||
|
|
|
@ -510,14 +510,12 @@ describe('RestApiClient', function() {
|
|||
describe('#setOptions', function() {
|
||||
/*Tests_SRS_NODE_IOTHUB_REST_API_CLIENT_18_003: [ `setOptions` shall call `this._http.setOptions` passing the same parameters ]*/
|
||||
it ('passes the options down', function(callback) {
|
||||
var fakeHttpRequestBuilder = { setOptions: sinon.stub().callsArg(1) };
|
||||
var fakeHttpRequestBuilder = { setOptions: sinon.stub() };
|
||||
var fakeOptions = '__FAKE_OPTIONS__';
|
||||
var client = new RestApiClient({host: 'host', sharedAccessSignature: 'sas'}, fakeAgent, fakeHttpRequestBuilder);
|
||||
client.setOptions(fakeOptions, function(err) {
|
||||
assert(!err);
|
||||
assert(fakeHttpRequestBuilder.setOptions.calledWith(fakeOptions));
|
||||
callback();
|
||||
});
|
||||
client.setOptions(fakeOptions);
|
||||
assert(fakeHttpRequestBuilder.setOptions.calledWith(fakeOptions));
|
||||
callback();
|
||||
});
|
||||
|
||||
});
|
||||
|
|
|
@ -203,9 +203,8 @@ export class MqttBase extends EventEmitter {
|
|||
/**
|
||||
* @private
|
||||
*/
|
||||
setOptions(options: any, callback: (err?: Error) => void): void {
|
||||
setOptions(options: any): void {
|
||||
this._options = options;
|
||||
callback();
|
||||
}
|
||||
|
||||
private _connectClient(callback: (err?: Error, connack?: any) => void): void {
|
||||
|
|
|
@ -155,14 +155,12 @@ describe('MqttBase', function () {
|
|||
var fakemqtt = new FakeMqtt();
|
||||
var transport = new MqttBase('test', fakemqtt);
|
||||
|
||||
transport.setOptions({ca: fakeCa}, function(err) {
|
||||
assert(!err);
|
||||
fakemqtt.connect = function(host, options) {
|
||||
assert.strictEqual(options.ca, fakeCa);
|
||||
done();
|
||||
};
|
||||
transport.connect(fakeConfig, function () {});
|
||||
});
|
||||
transport.setOptions({ca: fakeCa});
|
||||
fakemqtt.connect = function(host, options) {
|
||||
assert.strictEqual(options.ca, fakeCa);
|
||||
done();
|
||||
};
|
||||
transport.connect(fakeConfig, function () {});
|
||||
});
|
||||
|
||||
/*Tests_SRS_NODE_COMMON_MQTT_BASE_12_005: [The `connect` method shall call connect on MQTT.JS library and call the `done` callback with a `null` error object and the result as a second argument.]*/
|
||||
|
|
|
@ -321,26 +321,22 @@ export class Http extends EventEmitter implements Client.Transport {
|
|||
/*Codes_SRS_NODE_DEVICE_HTTP_16_010: [`setOptions` should not throw if `done` has not been specified.]*/
|
||||
/*Codes_SRS_NODE_DEVICE_HTTP_16_005: [If `done` has been specified the `setOptions` method shall call the `done` callback with no arguments when successful.]*/
|
||||
/*Codes_SRS_NODE_DEVICE_HTTP_16_009: [If `done` has been specified the `setOptions` method shall call the `done` callback with a standard javascript `Error` object when unsuccessful.]*/
|
||||
this._http.setOptions(options, (err) => {
|
||||
if (err) {
|
||||
if (done) done(err);
|
||||
} else {
|
||||
// setOptions used to exist both on Http and HttpReceiver with different options class. In order not to break backward compatibility we have
|
||||
// to check what properties this options object has to figure out what to do with it.
|
||||
if (options.hasOwnProperty('http') && options.http.hasOwnProperty('receivePolicy')) {
|
||||
/*Codes_SRS_NODE_DEVICE_HTTP_16_004: [The `setOptions` method shall call the `setOptions` method of the HTTP Receiver with the content of the `http.receivePolicy` property of the `options` parameter.]*/
|
||||
this._setReceiverOptions(options.http.receivePolicy);
|
||||
if (done) done(err);
|
||||
} else if (options.hasOwnProperty('interval')
|
||||
|| options.hasOwnProperty('at')
|
||||
|| options.hasOwnProperty('cron')
|
||||
|| options.hasOwnProperty('manualPolling')
|
||||
|| options.hasOwnProperty('drain')) {
|
||||
this._setReceiverOptions(options as any);
|
||||
if (done) done(err);
|
||||
}
|
||||
}
|
||||
});
|
||||
this._http.setOptions(options);
|
||||
|
||||
// setOptions used to exist both on Http and HttpReceiver with different options class. In order not to break backward compatibility we have
|
||||
// to check what properties this options object has to figure out what to do with it.
|
||||
if (options.hasOwnProperty('http') && options.http.hasOwnProperty('receivePolicy')) {
|
||||
/*Codes_SRS_NODE_DEVICE_HTTP_16_004: [The `setOptions` method shall call the `setOptions` method of the HTTP Receiver with the content of the `http.receivePolicy` property of the `options` parameter.]*/
|
||||
this._setReceiverOptions(options.http.receivePolicy);
|
||||
if (done) done();
|
||||
} else if (options.hasOwnProperty('interval')
|
||||
|| options.hasOwnProperty('at')
|
||||
|| options.hasOwnProperty('cron')
|
||||
|| options.hasOwnProperty('manualPolling')
|
||||
|| options.hasOwnProperty('drain')) {
|
||||
this._setReceiverOptions(options as any);
|
||||
if (done) done();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -35,7 +35,7 @@
|
|||
"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 94 --branches 85 --functions 93 --lines 96"
|
||||
"check-cover": "istanbul check-coverage --statements 94 --branches 83 --functions 93 --lines 96"
|
||||
},
|
||||
"engines": {
|
||||
"node": ">= 0.10"
|
||||
|
|
|
@ -476,23 +476,19 @@ export class Mqtt extends EventEmitter implements Client.Transport {
|
|||
/*Codes_SRS_NODE_DEVICE_MQTT_16_015: [The `setOptions` method shall throw an `ArgumentError` if the `cert` property is populated but the device uses symmetric key authentication.]*/
|
||||
if (this._authenticationProvider.type === AuthenticationType.Token && options.cert) throw new errors.ArgumentError('Cannot set x509 options on a device that uses token authentication.');
|
||||
|
||||
this._mqtt.setOptions(options, (err) => {
|
||||
this._mqtt.setOptions(options);
|
||||
|
||||
/*Codes_SRS_NODE_DEVICE_MQTT_16_069: [The `setOptions` method shall obtain the current credentials by calling `getDeviceCredentials` on the `AuthenticationProvider` passed to the constructor as an argument.]*/
|
||||
this._authenticationProvider.getDeviceCredentials((err, credentials) => {
|
||||
if (err) {
|
||||
/*Codes_SRS_NODE_DEVICE_MQTT_16_070: [The `setOptions` method shall call its callback with the error returned by `getDeviceCredentials` if it fails to return the credentials.]*/
|
||||
if (done) done(err);
|
||||
} else {
|
||||
/*Codes_SRS_NODE_DEVICE_MQTT_16_069: [The `setOptions` method shall obtain the current credentials by calling `getDeviceCredentials` on the `AuthenticationProvider` passed to the constructor as an argument.]*/
|
||||
this._authenticationProvider.getDeviceCredentials((err, credentials) => {
|
||||
if (err) {
|
||||
/*Codes_SRS_NODE_DEVICE_MQTT_16_070: [The `setOptions` method shall call its callback with the error returned by `getDeviceCredentials` if it fails to return the credentials.]*/
|
||||
if (done) done(err);
|
||||
} else {
|
||||
/*Codes_SRS_NODE_DEVICE_MQTT_16_012: [The `setOptions` method shall update the existing configuration of the MQTT transport with the content of the `options` object.]*/
|
||||
(this._authenticationProvider as X509AuthenticationProvider).setX509Options(options);
|
||||
/*Codes_SRS_NODE_DEVICE_MQTT_16_013: [If a `done` callback function is passed as a argument, the `setOptions` method shall call it when finished with no arguments.]*/
|
||||
/*Codes_SRS_NODE_DEVICE_MQTT_16_014: [The `setOptions` method shall not throw if the `done` argument is not passed.]*/
|
||||
if (done) done(null);
|
||||
}
|
||||
});
|
||||
/*Codes_SRS_NODE_DEVICE_MQTT_16_012: [The `setOptions` method shall update the existing configuration of the MQTT transport with the content of the `options` object.]*/
|
||||
(this._authenticationProvider as X509AuthenticationProvider).setX509Options(options);
|
||||
/*Codes_SRS_NODE_DEVICE_MQTT_16_013: [If a `done` callback function is passed as a argument, the `setOptions` method shall call it when finished with no arguments.]*/
|
||||
/*Codes_SRS_NODE_DEVICE_MQTT_16_014: [The `setOptions` method shall not throw if the `done` argument is not passed.]*/
|
||||
if (done) done(null);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
|
|
@ -1,83 +0,0 @@
|
|||
// Copyright (c) Microsoft. All rights reserved.
|
||||
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
|
||||
|
||||
'use strict';
|
||||
|
||||
var async = require('async');
|
||||
var uuid = require('uuid');
|
||||
var DeviceClient = require('azure-iot-device').Client;
|
||||
var Message = require('azure-iot-device').Message;
|
||||
var Mqtt = require('azure-iot-device-mqtt').Mqtt;
|
||||
|
||||
var sendingModuleConnectionString = process.env.IOT_MODULES_SENDING_MODULE;
|
||||
var sendingModuleOutputChannel = 'sendChannel';
|
||||
var receivingModuleConnectionString = process.env.IOT_MODULES_RECEIVING_MODULE;
|
||||
var receivingModuleInputChannel = 'receiveChannel';
|
||||
|
||||
var edgeModuleEventTransports = [ Mqtt] ;
|
||||
|
||||
edgeModuleEventTransports.forEach(function(Transport) {
|
||||
describe('Edge Module Events over ' + Transport.name, function() {
|
||||
var sendingClient;
|
||||
var receivingClient;
|
||||
|
||||
this.timeout(30000);
|
||||
|
||||
beforeEach(function(callback) {
|
||||
sendingClient = DeviceClient.fromConnectionString(sendingModuleConnectionString, Transport);
|
||||
receivingClient = DeviceClient.fromConnectionString(receivingModuleConnectionString, Transport);
|
||||
sendingClient.open(function(err) {
|
||||
if (err) {
|
||||
callback(err);
|
||||
} else {
|
||||
receivingClient.open(function(err) {
|
||||
if (err) {
|
||||
callback(err);
|
||||
} else {
|
||||
callback();
|
||||
}
|
||||
});
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(function(callback) {
|
||||
async.each([sendingClient, receivingClient], function(client, closeCallback) {
|
||||
if (client) {
|
||||
client.close(function() {
|
||||
closeCallback();
|
||||
});
|
||||
} else {
|
||||
closeCallback();
|
||||
}
|
||||
}, function(err) {
|
||||
sendingClient = null;
|
||||
receivingClient = null;
|
||||
callback(err);
|
||||
});
|
||||
});
|
||||
|
||||
it ('can send and receive single events', function(callback) {
|
||||
var testString = uuid.v4().toString();
|
||||
var testMessage = new Message(testString);
|
||||
|
||||
receivingClient.on('inputMessage', function(inputName, message) {
|
||||
receivingClient.complete(message, function(err) {
|
||||
if (err) {
|
||||
callback(err);
|
||||
} else {
|
||||
if (inputName === receivingModuleInputChannel && message.data.toString('ascii') === testString) {
|
||||
callback();
|
||||
}
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
sendingClient.sendOutputEvent(sendingModuleOutputChannel, testMessage, function(err) {
|
||||
if (err) {
|
||||
callback(err);
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
Загрузка…
Ссылка в новой задаче