Skip to content
This repository has been archived by the owner on Feb 11, 2020. It is now read-only.

Add option 'qos2Puback' -easy merge #642

Merged
merged 5 commits into from
May 19, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions examples/Server_With_All_Interfaces-Settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ var moscaSetting = {
{ type: "https", port: 3001, bundle: true, credentials: { keyPath: SECURE_KEY, certPath: SECURE_CERT } }
],
stats: false,
qos2Puback: false, // can set to true if using a client which will eat puback for QOS 2; e.g. mqtt.js
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking more about this, and I think this behavior should be a bit more customizable. How about adding it as a string with two options:'dropToQoS1','ignore'and'disconnect'? Maybe we can call it onQoS2publish`?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

an example of close of the connection is just above, so it's not going to take much serious thought :).
Are we worried about string comparison in such a well-called area of code?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not worry about it, it should not be a hot path: QoS 2 is not really supported.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I move the 'if' so it checks the QOS before anything else anyway. It's only here as an option to protect stupids like me who set mqtt.js to QoS2 :). (well node-red does default to QoS2 for mqtt subscription; fooled me).


logger: { name: 'MoscaServer', level: 'debug' },

Expand Down
13 changes: 11 additions & 2 deletions lib/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -536,14 +536,22 @@ Client.prototype.handleAuthorizePublish = function(err, success, packet) {
}

var dopuback = function() {
// if qos2Puback, then if qos 2, don't just ignore the message, puback it
// by converting internally to qos 1.
// this fools mqtt.js into not holding all messages forever
if (that.server.qos2Puback === true){
if (packet.qos === 2){
packet.qos = 1;
}
}

if (packet.qos === 1 && !(that._closed || that._closing)) {
that.connection.puback({
messageId: packet.messageId
});
}
};



// if success is passed as 'ignore', ack but don't publish.
if (success !== 'ignore'){
// publish message
Expand All @@ -552,6 +560,7 @@ Client.prototype.handleAuthorizePublish = function(err, success, packet) {
// ignore but acknowledge message
dopuback();
}

};

/**
Expand Down
8 changes: 6 additions & 2 deletions lib/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ function modernize(legacy) {
"stats",
"publishNewClient",
"publishClientDisconnect",
"publishSubscriptions"
"publishSubscriptions",
"qos2Puback"
];

// copy all conserved options
Expand Down Expand Up @@ -252,7 +253,8 @@ function validate(opts, validationOptions) {
'stats': { type: 'boolean' },
'publishNewClient': { type: 'boolean' },
'publishClientDisconnect': { type: 'boolean' },
'publishSubscriptions': { type: 'boolean' }
'publishSubscriptions': { type: 'boolean' },
'qos2Puback': { type: 'boolean' },
}
});

Expand Down Expand Up @@ -330,6 +332,7 @@ function defaultsLegacy() {
publishClientDisconnect: true,
publishSubscriptions: true,
maxInflightMessages: 1024,
qos2Puback: false,
logger: {
name: "mosca",
level: "warn",
Expand Down Expand Up @@ -366,6 +369,7 @@ function defaultsModern() {
publishClientDisconnect: true,
publishSubscriptions: true,
maxInflightMessages: 1024,
qos2Puback: false,
logger: {
name: "mosca",
level: "warn",
Expand Down
3 changes: 3 additions & 0 deletions lib/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,9 @@ function Server(opts, callback) {

var that = this;

// put QOS-2 spoofing as a variable direct on server
this.qos2Puback = this.modernOpts.qos2Puback;

// each Server has a dummy id for logging purposes
this.id = this.modernOpts.id || shortid.generate();

Expand Down
84 changes: 84 additions & 0 deletions test/abstract_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -956,6 +956,90 @@ module.exports = function(moscaSettings, createConnection) {
});
});

it("should by default not puback client publish to QOS 2", function(done) {
var onPublishedCalled = false;
var clientId;
var count = 0;
var timer;

instance.published = function(packet, serverClient, callback) {
onPublishedCalled = true;
expect(packet.topic).to.be.equal("testQOS2");
callback();
};

buildAndConnect(done, function(client) {
clientId = client.opts.clientId;

client.publish({
messageId: 42,
topic: "testQOS2",
payload: "publish expected",
qos: 2
});

// allow 1 second to hear puback
timer = setTimeout(function(){
client.disconnect();
}, 1000);

// default QOS 2 should NOT puback
client.on("puback", function() {
count++;
//expect(count).to.eql(1);
client.disconnect();
});
client.on("close", function() {
expect(count).to.eql(0);
client.disconnect();
clearTimeout(timer);
});
});
});


it("should optionally (.qos2Puback) puback client publish to QOS 2", function(done) {
var onPublishedCalled = false;
var clientId;
var count = 0;
var timer;

instance.qos2Puback = true;
instance.published = function(packet, serverClient, callback) {
onPublishedCalled = true;
expect(packet.topic).to.be.equal("testQOS2");
callback();
};

buildAndConnect(done, function(client) {
clientId = client.opts.clientId;

client.publish({
messageId: 42,
topic: "testQOS2",
payload: "publish expected",
qos: 2
});

// allow 1 second to hear puback
timer = setTimeout(function(){
client.disconnect();
}, 1000);

// with maxqos=1, QOS 2 should puback
client.on("puback", function() {
count++;
expect(count).to.eql(1);
client.disconnect();
});
client.on("close", function() {
expect(count).to.eql(1);
client.disconnect();
clearTimeout(timer);
});
});
});

it("should emit an event when a new client is connected", function(done) {
buildClient(done, function(client) {

Expand Down