From b5dfd62c992b186203368ec14a15965f31201b74 Mon Sep 17 00:00:00 2001 From: Tim Janke Date: Mon, 27 Feb 2023 12:21:39 +0100 Subject: [PATCH 1/5] Add an option to not unsubscribe topics on disconnect --- .../@node-red/nodes/core/network/10-mqtt.html | 19 +++++++++++++++++++ .../@node-red/nodes/core/network/10-mqtt.js | 19 ++++++++++++++----- .../@node-red/nodes/locales/de/messages.json | 1 + .../nodes/locales/en-US/messages.json | 1 + 4 files changed, 35 insertions(+), 5 deletions(-) diff --git a/packages/node_modules/@node-red/nodes/core/network/10-mqtt.html b/packages/node_modules/@node-red/nodes/core/network/10-mqtt.html index bb76b33bc..c89550dc6 100644 --- a/packages/node_modules/@node-red/nodes/core/network/10-mqtt.html +++ b/packages/node_modules/@node-red/nodes/core/network/10-mqtt.html @@ -249,6 +249,12 @@ +
+ +
@@ -505,6 +511,7 @@ label: RED._("node-red:mqtt.label.keepalive"), validate:RED.validators.number(false)}, cleansession: {value: true}, + unsubscribeOnDisconnect: {value: true}, birthTopic: {value:"", validate:validateMQTTPublishTopic}, birthQos: {value:"0"}, birthRetain: {value:"false"}, @@ -620,6 +627,10 @@ this.cleansession = true; $("#node-config-input-cleansession").prop("checked",true); } + if (typeof this.unsubscribeOnDisconnect === 'undefined') { + this.unsubscribeOnDisconnect = true; + $("#node-config-input-unsubscribeOnDisconnect").prop("checked",true); + } if (typeof this.usetls === 'undefined') { this.usetls = false; $("#node-config-input-usetls").prop("checked",false); @@ -635,6 +646,14 @@ if (typeof this.protocolVersion === 'undefined') { this.protocolVersion = 4; } + $("#node-config-input-cleansession").on("change", function() { + var useCleanSession = $("#node-config-input-cleansession").is(':checked'); + if(useCleanSession) { + $("div.form-row.mqtt-persistence").hide(); + } else { + $("div.form-row.mqtt-persistence").show(); + } + }); $("#node-config-input-protocolVersion").on("change", function() { var v5 = $("#node-config-input-protocolVersion").val() == "5"; if(v5) { diff --git a/packages/node_modules/@node-red/nodes/core/network/10-mqtt.js b/packages/node_modules/@node-red/nodes/core/network/10-mqtt.js index 0a7ed6dfa..6f757a4c0 100644 --- a/packages/node_modules/@node-red/nodes/core/network/10-mqtt.js +++ b/packages/node_modules/@node-red/nodes/core/network/10-mqtt.js @@ -482,6 +482,7 @@ module.exports = function(RED) { setIfHasProperty(opts, node, "protocolVersion", init); setIfHasProperty(opts, node, "keepalive", init); setIfHasProperty(opts, node, "cleansession", init); + setIfHasProperty(opts, node, "unsubscribeOnDisconnect", init); setIfHasProperty(opts, node, "topicAliasMaximum", init); setIfHasProperty(opts, node, "maximumPacketSize", init); setIfHasProperty(opts, node, "receiveMaximum", init); @@ -590,6 +591,9 @@ module.exports = function(RED) { if (typeof node.cleansession === 'undefined') { node.cleansession = true; } + if (typeof node.unsubscribeOnDisconnect === 'undefined') { + node.unsubscribeOnDisconnect = true; + } //use url or build a url from usetls://broker:port if (node.url && node.brokerurl !== node.url) { @@ -660,6 +664,7 @@ module.exports = function(RED) { node.options.password = node.password; node.options.keepalive = node.keepalive; node.options.clean = node.cleansession; + node.options.unsubscribeOnDisconnect = node.unsubscribeOnDisconnect; node.options.clientId = node.clientid || 'nodered_' + RED.util.generateId(); node.options.reconnectPeriod = RED.settings.mqttReconnectTime||5000; delete node.options.protocolId; //V4+ default @@ -1228,12 +1233,16 @@ module.exports = function(RED) { node.on('close', function(removed, done) { if (node.brokerConn) { if(node.isDynamic) { - Object.keys(node.dynamicSubs).forEach(function (topic) { - node.brokerConn.unsubscribe(topic, node.id, removed); - }); - node.dynamicSubs = {}; + if (node.brokerConn.options.unsubscribeOnDisconnect) { + Object.keys(node.dynamicSubs).forEach(function (topic) { + node.brokerConn.unsubscribe(topic, node.id, removed); + }); + node.dynamicSubs = {}; + } } else { - node.brokerConn.unsubscribe(node.topic,node.id, removed); + if (node.brokerConn.options.unsubscribeOnDisconnect) { + node.brokerConn.unsubscribe(node.topic, node.id, removed); + } } node.brokerConn.deregister(node, done, removed); node.brokerConn = null; diff --git a/packages/node_modules/@node-red/nodes/locales/de/messages.json b/packages/node_modules/@node-red/nodes/locales/de/messages.json index 65f251e98..9d6b927ac 100644 --- a/packages/node_modules/@node-red/nodes/locales/de/messages.json +++ b/packages/node_modules/@node-red/nodes/locales/de/messages.json @@ -362,6 +362,7 @@ "port": "Port", "keepalive": "Keep-Alive", "cleansession": "Bereinigte Sitzung (clean session) verwenden", + "unsubscribeOnDisconnect": "Abonnement bei Verbindungsende automatisch beenden", "cleanstart": "Verwende bereinigten Start", "use-tls": "TLS", "tls-config": "TLS-Konfiguration", diff --git a/packages/node_modules/@node-red/nodes/locales/en-US/messages.json b/packages/node_modules/@node-red/nodes/locales/en-US/messages.json index 66c492dc8..70bc1b888 100644 --- a/packages/node_modules/@node-red/nodes/locales/en-US/messages.json +++ b/packages/node_modules/@node-red/nodes/locales/en-US/messages.json @@ -414,6 +414,7 @@ "port": "Port", "keepalive": "Keep Alive", "cleansession": "Use clean session", + "unsubscribeOnDisconnect": "Automatically unsubscribe when disconnecting", "cleanstart": "Use clean start", "use-tls": "Use TLS", "tls-config": "TLS Configuration", From 8dcc530f44a39ae8c4bd2f85a515305e24993765 Mon Sep 17 00:00:00 2001 From: Tim Janke Date: Mon, 27 Feb 2023 12:41:29 +0100 Subject: [PATCH 2/5] Refactor to use the already existing autoUnsubscribe property The tests had an unused property called autoUnsubscribe. I refactored the code to use this wording too. --- .../@node-red/nodes/core/network/10-mqtt.html | 14 +++++++------- .../@node-red/nodes/core/network/10-mqtt.js | 12 ++++++------ .../@node-red/nodes/locales/de/messages.json | 2 +- .../@node-red/nodes/locales/en-US/messages.json | 2 +- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/packages/node_modules/@node-red/nodes/core/network/10-mqtt.html b/packages/node_modules/@node-red/nodes/core/network/10-mqtt.html index c89550dc6..0bb1e12f8 100644 --- a/packages/node_modules/@node-red/nodes/core/network/10-mqtt.html +++ b/packages/node_modules/@node-red/nodes/core/network/10-mqtt.html @@ -250,9 +250,9 @@
-
@@ -511,7 +511,7 @@ label: RED._("node-red:mqtt.label.keepalive"), validate:RED.validators.number(false)}, cleansession: {value: true}, - unsubscribeOnDisconnect: {value: true}, + autoUnsubscribe: {value: true}, birthTopic: {value:"", validate:validateMQTTPublishTopic}, birthQos: {value:"0"}, birthRetain: {value:"false"}, @@ -627,9 +627,9 @@ this.cleansession = true; $("#node-config-input-cleansession").prop("checked",true); } - if (typeof this.unsubscribeOnDisconnect === 'undefined') { - this.unsubscribeOnDisconnect = true; - $("#node-config-input-unsubscribeOnDisconnect").prop("checked",true); + if (typeof this.autoUnsubscribe === 'undefined') { + this.autoUnsubscribe = true; + $("#node-config-input-autoUnsubscribe").prop("checked",true); } if (typeof this.usetls === 'undefined') { this.usetls = false; diff --git a/packages/node_modules/@node-red/nodes/core/network/10-mqtt.js b/packages/node_modules/@node-red/nodes/core/network/10-mqtt.js index 6f757a4c0..9e17463dd 100644 --- a/packages/node_modules/@node-red/nodes/core/network/10-mqtt.js +++ b/packages/node_modules/@node-red/nodes/core/network/10-mqtt.js @@ -482,7 +482,7 @@ module.exports = function(RED) { setIfHasProperty(opts, node, "protocolVersion", init); setIfHasProperty(opts, node, "keepalive", init); setIfHasProperty(opts, node, "cleansession", init); - setIfHasProperty(opts, node, "unsubscribeOnDisconnect", init); + setIfHasProperty(opts, node, "autoUnsubscribe", init); setIfHasProperty(opts, node, "topicAliasMaximum", init); setIfHasProperty(opts, node, "maximumPacketSize", init); setIfHasProperty(opts, node, "receiveMaximum", init); @@ -591,8 +591,8 @@ module.exports = function(RED) { if (typeof node.cleansession === 'undefined') { node.cleansession = true; } - if (typeof node.unsubscribeOnDisconnect === 'undefined') { - node.unsubscribeOnDisconnect = true; + if (typeof node.autoUnsubscribe === 'undefined') { + node.autoUnsubscribe = true; } //use url or build a url from usetls://broker:port @@ -664,7 +664,7 @@ module.exports = function(RED) { node.options.password = node.password; node.options.keepalive = node.keepalive; node.options.clean = node.cleansession; - node.options.unsubscribeOnDisconnect = node.unsubscribeOnDisconnect; + node.options.autoUnsubscribe = node.autoUnsubscribe; node.options.clientId = node.clientid || 'nodered_' + RED.util.generateId(); node.options.reconnectPeriod = RED.settings.mqttReconnectTime||5000; delete node.options.protocolId; //V4+ default @@ -1233,14 +1233,14 @@ module.exports = function(RED) { node.on('close', function(removed, done) { if (node.brokerConn) { if(node.isDynamic) { - if (node.brokerConn.options.unsubscribeOnDisconnect) { + if (node.brokerConn.options.autoUnsubscribe) { Object.keys(node.dynamicSubs).forEach(function (topic) { node.brokerConn.unsubscribe(topic, node.id, removed); }); node.dynamicSubs = {}; } } else { - if (node.brokerConn.options.unsubscribeOnDisconnect) { + if (node.brokerConn.options.autoUnsubscribe) { node.brokerConn.unsubscribe(node.topic, node.id, removed); } } diff --git a/packages/node_modules/@node-red/nodes/locales/de/messages.json b/packages/node_modules/@node-red/nodes/locales/de/messages.json index 9d6b927ac..4ac593504 100644 --- a/packages/node_modules/@node-red/nodes/locales/de/messages.json +++ b/packages/node_modules/@node-red/nodes/locales/de/messages.json @@ -362,7 +362,7 @@ "port": "Port", "keepalive": "Keep-Alive", "cleansession": "Bereinigte Sitzung (clean session) verwenden", - "unsubscribeOnDisconnect": "Abonnement bei Verbindungsende automatisch beenden", + "autoUnsubscribe": "Abonnement bei Verbindungsende automatisch beenden", "cleanstart": "Verwende bereinigten Start", "use-tls": "TLS", "tls-config": "TLS-Konfiguration", diff --git a/packages/node_modules/@node-red/nodes/locales/en-US/messages.json b/packages/node_modules/@node-red/nodes/locales/en-US/messages.json index 70bc1b888..801aeace8 100644 --- a/packages/node_modules/@node-red/nodes/locales/en-US/messages.json +++ b/packages/node_modules/@node-red/nodes/locales/en-US/messages.json @@ -414,7 +414,7 @@ "port": "Port", "keepalive": "Keep Alive", "cleansession": "Use clean session", - "unsubscribeOnDisconnect": "Automatically unsubscribe when disconnecting", + "autoUnsubscribe": "Automatically unsubscribe when disconnecting", "cleanstart": "Use clean start", "use-tls": "Use TLS", "tls-config": "TLS Configuration", From 182361c176257f5135528d8d086fd55d3be0a509 Mon Sep 17 00:00:00 2001 From: Tim Janke Date: Mon, 27 Feb 2023 12:43:04 +0100 Subject: [PATCH 3/5] Re-enable the tests for the autoUnsubscribe property --- test/nodes/core/network/21-mqtt_spec.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/nodes/core/network/21-mqtt_spec.js b/test/nodes/core/network/21-mqtt_spec.js index f97442b50..16c38d2e5 100644 --- a/test/nodes/core/network/21-mqtt_spec.js +++ b/test/nodes/core/network/21-mqtt_spec.js @@ -53,7 +53,7 @@ describe('MQTT Nodes', function () { mqttBroker.should.have.property('broker', BROKER_HOST); mqttBroker.should.have.property('port', BROKER_PORT); mqttBroker.should.have.property('brokerurl'); - // mqttBroker.should.have.property('autoUnsubscribe', true);//default: true + mqttBroker.should.have.property('autoUnsubscribe', true); //default: true mqttBroker.should.have.property('autoConnect', false);//Set "autoConnect:false" in brokerOptions mqttBroker.should.have.property('options'); mqttBroker.options.should.have.property('clean', true); @@ -96,8 +96,8 @@ describe('MQTT Nodes', function () { mqttBroker.should.have.property('broker', BROKER_HOST); mqttBroker.should.have.property('port', BROKER_PORT); mqttBroker.should.have.property('brokerurl'); - // mqttBroker.should.have.property('autoUnsubscribe', true);//default: true - mqttBroker.should.have.property('autoConnect', false);//Set "autoConnect:false" in brokerOptions + mqttBroker.should.have.property('autoUnsubscribe', true); + mqttBroker.should.have.property('autoConnect', false); //Set "autoConnect:false" in brokerOptions mqttBroker.should.have.property('options'); mqttBroker.options.should.have.property('clean', false); mqttBroker.options.should.have.property('clientId', 'clientid'); From c94f0896e190276480d996986a9f1b349ed24f99 Mon Sep 17 00:00:00 2001 From: Tim Janke Date: Mon, 27 Feb 2023 14:07:36 +0100 Subject: [PATCH 4/5] Ensure that a client id is set if autoUnsubscribe is disabled --- .../@node-red/nodes/core/network/10-mqtt.html | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/packages/node_modules/@node-red/nodes/core/network/10-mqtt.html b/packages/node_modules/@node-red/nodes/core/network/10-mqtt.html index 0bb1e12f8..5b1b1f799 100644 --- a/packages/node_modules/@node-red/nodes/core/network/10-mqtt.html +++ b/packages/node_modules/@node-red/nodes/core/network/10-mqtt.html @@ -489,17 +489,23 @@ tls: {type:"tls-config",required: false, label:RED._("node-red:mqtt.label.use-tls") }, clientid: {value:"", validate: function(v, opt) { - var ok = false; + let ok = true; if ($("#node-config-input-clientid").length) { // Currently editing the node - ok = $("#node-config-input-cleansession").is(":checked") || (v||"").length > 0; + let needClientId = !$("#node-config-input-cleansession").is(":checked") || !$("#node-config-input-autoUnsubscribe").is(":checked") + if (needClientId) { + ok = (v||"").length > 0; + } } else { - ok = (this.cleansession===undefined || this.cleansession) || (v||"").length > 0; + let needClientId = !(this.cleansession===undefined || this.cleansession) || this.autoUnsubscribe; + if (needClientId) { + ok = (v||"").length > 0; + } } - if (ok) { - return ok; + if (!ok) { + return RED._("node-red:mqtt.errors.invalid-client-id"); } - return RED._("node-red:mqtt.errors.invalid-client-id"); + return true; }}, autoConnect: {value: true}, usetls: {value: false}, From e7617de1eeadfe95e03070b90b4f6b4ef44f5853 Mon Sep 17 00:00:00 2001 From: Tim Janke Date: Mon, 13 Mar 2023 13:22:51 +0100 Subject: [PATCH 5/5] Replace a `var` with a `const` since its value won't be modified further on --- packages/node_modules/@node-red/nodes/core/network/10-mqtt.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/node_modules/@node-red/nodes/core/network/10-mqtt.html b/packages/node_modules/@node-red/nodes/core/network/10-mqtt.html index 5b1b1f799..009076cf9 100644 --- a/packages/node_modules/@node-red/nodes/core/network/10-mqtt.html +++ b/packages/node_modules/@node-red/nodes/core/network/10-mqtt.html @@ -653,7 +653,7 @@ this.protocolVersion = 4; } $("#node-config-input-cleansession").on("change", function() { - var useCleanSession = $("#node-config-input-cleansession").is(':checked'); + const useCleanSession = $("#node-config-input-cleansession").is(':checked'); if(useCleanSession) { $("div.form-row.mqtt-persistence").hide(); } else {