telefonicaid / iotagent-node-lib

Module to enable IoT Agent developers to build custom agents for their devices that can easily connect to NGSI Context Brokers
https://iotagent-node-lib.rtfd.io/
GNU Affero General Public License v3.0
59 stars 84 forks source link

Bidirectional plugin doesn't perform unsubscription on device removal #576

Open fgalan opened 6 years ago

fgalan commented 6 years ago

The library has methods that implementes the unsubscribe operation on CB. In particular, the unsubscribe() function at https://github.com/telefonicaid/iotagent-node-lib/blob/master/lib/services/ngsi/subscriptionService.js#L184

function unsubscribe(device, id, callback) {
    ...
    if (device.cbHost) {
        options.uri = 'http://' + device.cbHost + '/v1/unsubscribeContext';
    } else {
        options.uri = 'http://' + config.getConfig().contextBroker.host + ':' + config.getConfig().contextBroker.port +
            '/v1/unsubscribeContext';
    }
...
}

which is called by removeAllSubscriptions() function at https://github.com/telefonicaid/iotagent-node-lib/blob/master/lib/services/devices/deviceService.js#L415:

function removeAllSubscriptions(device, callback) {
    function removeSubscription(subscription, callback) {
        subscriptions.unsubscribe(device, subscription.id, callback);
    }

    if (device.subscriptions) {
        async.map(device.subscriptions, removeSubscription, callback);
    } else {
        callback(null, {});
    }
}

However, nobody in the library is using this method, as can be checked by this search:

In addition, IOTA agents hasn't any link with this code in the library, I mean nobody using it the IOTA JSON agent:

and nobody using in the IOTA UL agent:

If my theory confirms (I may be wrong) it seems that unsubcription code in the library is dead code. As a consecuence, subscriptions are not correctly cleaned up when devices are deleted by API, leaving dirty stuff in CB.

chicco785 commented 6 years ago

i can also add that when you delete a service with device=true no device is actually deleted contrary to what the API description states

fgalan commented 6 years ago

i can also add that when you delete a service with device=true no device is actually deleted contrary to what the API description states

Only in sceanrios involving bidirectionality plugin? Or always?

In the second case, maybe it should be moved to an independent issue.

chicco785 commented 6 years ago

i have to check

Sunny-NEC commented 2 years ago

@fgalan sir I am looking in this issue. as we saw its an old issue , i really want to know it is good to go or not.

fgalan commented 2 years ago

@fgalan sir I am looking in this issue. as we saw its an old issue , i really want to know it is good to go or not.

The issue is old, but under my understanding it's still valid for working on it. That for your willingness!

fgalan commented 2 years ago

PR #1082

Sunny-NEC commented 2 years ago

remove the removeAllSubscriptions function but as the community said this is not the solution for this issue. so I am going to drop this issue .

fgalan commented 2 years ago

remove the removeAllSubscriptions function but as the community said this is not the solution for this issue. so I am going to drop this issue .

@Sunny-NEC I don't understand what you mean by "as the community sais this is not the solution". Who is "the community" in this context? Could you elaborate a little bit, please?

Sunny-NEC commented 2 years ago

@fgalan I am taking the reference https://github.com/telefonicaid/iotagent-node-lib/pull/1082#issuecomment-906161732 if their is any other approach for this issue please let me know :) .

fgalan commented 2 years ago

@fgalan I am taking the reference #1082 (comment) if their is any other approach for this issue please let me know :) .

Well, the solution is the one I describe in that comment you cite ;)

So the solution shouldn't be remove the unsubscription code in the library but using it properly in the user code (IOTA agents code) to clean up subscriptions when devices are deleted so no "zombie" subscription remains at CB after the deletion.

I think is pretty clear, but don't hesitate to ask if you don't understand something, please.