migrated from Bugzilla #407605
status RESOLVED severity enhancement in component MQTT-JS for ---
Reported in version v0.5 on platform All
Assigned to: Nicholas O'Leary
On 2013-05-08 19:47:40 -0400, Simon Racliffe wrote:
Created attachment 230690
Patch to implement url's as parameters
Currently the mqtt-js client uses a host,port parameter to specify the websocket server and you have to specify useSSL if you want to use SSL. Also the path on the server is hardcoded in the code.
A better way to specify this is to simply to pass in a url in the form:
{ws:|wss:}//hostname[:port]/path
The attached patch changes the implementation to do it this way.
Regards
Simon Ratcliffe
On 2013-05-10 11:56:23 -0400, Andy Piper wrote:
Simon, thanks very much for your patch! We will take a look.
In order for us to accept your contribution to the project, can you please confirm:
That you authored 100% of the content youâre contributing?
That you have the rights to contribute this content to Eclipse?
That you're willing to contribute the content under the projectâs EPL and EDL license(s)
In addition, since this patch is (just!) greater than 250 lines, we will need to pass it through the IP review process. CQ # 7253 opened for this review.
On 2013-05-10 11:58:05 -0400, Andy Piper wrote:
Assigning to Andrew.
On 2013-05-10 19:56:29 -0400, Simon Racliffe wrote:
Andy, no problem. I 100% authored the contribution and have the rights to contribute it.
I am willing to contribute this under the projects licence's.
Regards
Simon Ratcliffe
ProAtomic Software Development
On 2013-05-17 07:45:35 -0400, Simon Racliffe wrote:
Hi guys,
Any progress on this one?
I think it is important to address this as it is a change to the API and therefore a good idea to say yay or nay before many people start to use the library.
I know that it got tagged as needing to be reviewed due to the size, but most of the patch was just the references to the surrounding lines. I think a philosophical decision/discussion on the API change is important.
Regards
Simon Ratcliffe
On 2013-05-17 08:03:10 -0400, Nicholas O'Leary wrote:
Hi Simon,
Philosophically, I'm all for allowing the url to be passed in rather than just the hostname/port.
A related part of this is that the client assumes the path to connect to is always "/mqtt". This change would allow that to be anything - again, in my view a good thing.
I haven't looked at your specific implementation, but I don't see why this needs to be an API change - it could be added as an alternative acceptable parameter to the existing constructor.
Ultimately this needs input from Andy Banks.
On 2013-05-17 08:15:53 -0400, Simon Racliffe wrote:
I agree it doesn't have to be an API change, but the current implementation is restrictive and cumbersome. Maintaining multiple alternative API's when not providing real functionality is not a good idea in my opinion. Also the library does no checking of the parameters and just concatenates them and passes them on, so there is no benefit to passing them separately.
Once the other issue I have reported is addressed (bug 408078) any error in the url will be handled correctly.
On 2013-05-18 06:22:46 -0400, Andrew Banks wrote:
I chose to use host,port and the useSSL flag rather than requiring the user to provide a URL because its less error prone and requires less knowledge of the correct URL format. Whether one needs the extra flexibility of providing a URL vs the constraints of generating only correct URLs is moot, what would be undesirable is to have two ways of doing this.
There is an open issue for the Oasis Technical Committee to resolve on the
path name and how WebSockets fits with MQTT in general, so I don't want to preempt the outcome of that now. See
On 2013-05-24 08:06:57 -0400, Simon Racliffe wrote:
Hi Andrew,
I doubt that anyone that wants to use mqtt-js is not going to understand how to construct a URL (scary, but most 5yo's know what a URL is). I didn't see anything of relevance in the topic you posted.
If you are going to parameterise the URL it needs to contain all options.
Just my thoughts.
On 2013-05-24 08:23:14 -0400, Andy Piper wrote:
CQ 7253 was approved, but the header needs amending to include Simon if the patch is applied.
On 2013-07-22 09:31:43 -0400, Al Stockdill-Mander wrote:
Created attachment 233669
Allow the use of URIs and host/port
This patch allows the JS client to be used by providing both host/port or with a URI.
Something to be aware of; if you were using it in host/port and have data in the local storage for the client, it would no longer be visible to the client if you change to using a URI.
On 2013-09-27 16:27:34 -0400, Nicholas O'Leary wrote:
Fix pushed to the develop branch.
The fix is completely backward compatible with the existing api.
The constructor signature is now:
Client(host,[port,[path]],clientId)
where:
host: is the hostname/ip address, or the fully qualified ws uri
port: if host is not a uri, this is the port to connect to
path: if host is not a uri, this is the path portion, defaults to /mqtt
The other way the client supports supplying server details is via the ConnectOptions.hosts array passed to the connect function. This can now be an array of fully qualified ws uris. If they are determined to be uris, the .ports array is ignored. For better or worse, as before, if .hosts/.ports is used, you must still provide host details in the constructor to provide a key into local storage persistence.
On 2013-09-30 04:19:41 -0400, Andrew Banks wrote:
Nick, please be aware that there are data migration issues with this fix which may
cause data loss. If you flip between the host+port version of the constructor and
the url version, or if you use differing forms of the url to reach the same server
then different sets of stored data will be used for recovery, causing duplicate or
lost messages.
On 2013-09-30 05:59:48 -0400, Nicholas O'Leary wrote:
Andy,
yes, if a user modifies how they specify the broker, it does risk them losing state.
But that scenario already existed with the client; if a broker had multiple CNAMES and a user flipped between them, exactly the same would happen. So this fix does not introduce data loss scenarios that weren't already there.
In reality, application isn't going to suddenly flip from one method to the other.
Given it could already happen, I assume the documentation is clear on this point - if not, it should be updated.
On 2013-09-30 06:01:15 -0400, Nicholas O'Leary wrote:
Also, the fix ensures that an existing application that upgraded, with no changes to the application code, would still access the same local storage as before - so there is no data migration issue there.
migrated from Bugzilla #407605 status RESOLVED severity enhancement in component MQTT-JS for --- Reported in version v0.5 on platform All Assigned to: Nicholas O'Leary
Original attachment names and IDs:
On 2013-05-08 19:47:40 -0400, Simon Racliffe wrote:
On 2013-05-10 11:56:23 -0400, Andy Piper wrote:
On 2013-05-10 11:58:05 -0400, Andy Piper wrote:
On 2013-05-10 19:56:29 -0400, Simon Racliffe wrote:
On 2013-05-17 07:45:35 -0400, Simon Racliffe wrote:
On 2013-05-17 08:03:10 -0400, Nicholas O'Leary wrote:
On 2013-05-17 08:15:53 -0400, Simon Racliffe wrote:
On 2013-05-18 06:22:46 -0400, Andrew Banks wrote:
On 2013-05-24 08:06:57 -0400, Simon Racliffe wrote:
On 2013-05-24 08:23:14 -0400, Andy Piper wrote:
On 2013-07-22 09:31:43 -0400, Al Stockdill-Mander wrote:
On 2013-09-27 16:27:34 -0400, Nicholas O'Leary wrote:
On 2013-09-30 04:19:41 -0400, Andrew Banks wrote:
On 2013-09-30 05:59:48 -0400, Nicholas O'Leary wrote:
On 2013-09-30 06:01:15 -0400, Nicholas O'Leary wrote: