Add support for removing a single subscription handler

[iche033]

Desired behavior

Currently there is one bool Node::Unsusbcribe(const std::string &_topic) API which removes all subscription handlers for that topic in the Node. The feature request is to have a variant of the Unsubscribe API that supports removing just one specific subscription handler from the node.

As an example, the current usage of Subscribe / Unsubscribe and their behavior is as shown below:

// create one subscription
bool success1 = node.Subscribe(myTopic, callback1, opts);
// create another subscription
bool success2 = node.Subscribe(myTopic, callback2, opts);
// unsubscribes both callback1 and callback2
node.Unsubscribe(topic);

Ideally we can have something like:

// create one subscription
Subscriber sub1 = node.Subscribe(myTopic, callback1, opts);
// create another subscription
Subscriber sub2 = node.Subscribe(myTopic, callback2, opts);
// unsubscribes just sub1 
node.Unsubscribe(sub1);
// alternative API - unsubscribes just sub2 
sub2.Unsubscribe();

The new APIs above is just for illustrative purposes. It is however a breaking change since it changes the return type from bool to a custom struct / class.

To minimize breaking changes, another option would be to add overloaded Subscribe(myTopic, callback1, opts, sub1) API in which you can pass a reference for it to be set by the function.

Alternatives considered

See above for alternative API suggestions

Implementation suggestion

For Unsubscribing a single subscription from node, we'll need a function similar to Node::Unsubscribe. The difference being:

• Support removing handlers by a unique id
  

  gz-transport/src/Node.cc

  Line 615 in 3caed76

  if (!this->dataPtr->RemoveHandlersFromPubQueue(topic))

• Only notify publishers that the node is no longer interested in receiving published messages if no more handlers exists:

  gz-transport/src/Node.cc

  Line 644 in 3caed76

  // Notify to the publishers that I am no longer interested in the topic.

[caguero]

What about something like this:

Subscriber sub1 = node.Subscribe(myTopic, callback1, opts);

And then, no explicit Unsubscribe(), you just unsubscribe when sub1 goes out of scope and it's destroyed.

[iche033]

yep that sounds good. Since this changes the return type, I think it may not be trivial to deprecate the existing Subscribe functions and go through a tick-tock cycle. So this will break people's code.

What do you think about offering this through a non-breaking way. One alternative is to have a complete separate function, e.g.

    // just brainstorming here
    Subscriber CreateSubscriber(topic, callback, opts);
    Subscriber CreateSubscriptionHandler(topic, callback, opts);

maybe we could do something to forward the args to existing Subscribe functions so we don't have to overload the new function with different callback variants.

[caguero]

sub1 = node.CreateSubscriber(topic, callback, opts) looks very reasonable to me.

Another question, what should be the expected behavior if somebody calls node.Unsubscribe(topic) after having subscribed to topic with a CreateSubscriber(topic,...). I can see two options:

1. Nothings happens. The only way to unsubscribe to topic is destroying sub1.
2. Unsubscribe from topic. Then, even if sub1 is still in scope, no callbacks will be executed.

[iche033]

for me, I would expect 2. to happen. The functionality of Node::Unsubscribe(const std::string &_topic) will remain the same, which is that it will remove all subscriptions, use cases:

bool result = node.Subscribe(topic, callback, opts);
Subscriber sub1 = node.CreateSubscriber(topic, callback2, opts);
node.Unsubscribe(topic); // removes both subscriptions and sub1 no longer receives callback

bool result = node.Subscribe(topic, callback, opts);
{
  Subscriber sub1 = node.CreateSubscriber(topic, callback2, opts);
  // sub1 will be unsubscribed once it goes out of scope
}
node.Unsubscribe(topic); // removes remaining (first) subscription