don't emit unsubscribe on client close

[behrad]

No description provided.

[mcollina]

Good job, can you add a unit test so we do not regress?

[coveralls]

Coverage remained the same at 97.451% when pulling 39569cf on behrad:unsubscribe into 18eff9f on mcollina:master.

[behrad]

sure 👍

[behrad]

covering test added @mcollina

[coveralls]

Coverage remained the same at 97.451% when pulling 5bea9df on behrad:unsubscribe into 18eff9f on mcollina:master.

[ilvalle]

This PR blocks any logic a custom broker can run based on clients' unsubscriptions. Which is the advantage in not firing it? And without reverting this, is there any other way to catch client unsubscriptions on close event?

[GavinDmello]

This PR just blocks unsubscriptions on disconnects. If normal unsubscribe events are not being fired then it's an issue

[mcollina]

Ahum, I think we might have to revert this, at least partially.

IMHO we should fire 'unsubscribe' for clean = true clients, but not for clean = false  client. Would that still work for you @behrad?

[behrad]

we should fire 'unsubscribe' for clean = true clients, but not for clean = false  client. Would that still work for you @behrad?

Yes, I believe we should fire unsubscribe only for clean=true clients.

[mcollina]

@behrad would you mind firing a PR?