Cases with incorrect behavior of callbacks for presence subscriptions

[dargolith]

I have been using the presence api some lately and found some things that puzzled me.

Using the current latest npm versions: deepstream version: 3.1.2 deepstream.io-client-js version: 2.3.0

When using the two different variation of presence subscriptions (with username and without) the callback arguments are in different order.

const DeepstreamServer = require('deepstream.io');
const DeepstreamClient = require('deepstream.io-client-js');

const subCallback = type => (arg1, arg2) => {
  console.log('subCallback', type, 'arg1:', arg1, 'arg2:', arg2);
};

const server = new DeepstreamServer();
server.start();

const client = new DeepstreamClient('localhost:6020');
client.login({ username: 'testClient', password: 'password' }, success => {
  if (success) {
    client.presence.subscribe('testClient2', subCallback('withId'));
    client.presence.subscribe(subCallback('withoutId'));
    const client2 = new DeepstreamClient('localhost:6020');
    client2.login({ username: 'testClient2', password: 'password' });
  }
});

This gives the following output:

subCallback withoutId arg1: testClient2 arg2: true
subCallback withId arg1: true arg2: testClient2
subCallback withoutId arg1: testClient2 arg2: true
subCallback withId arg1: true arg2: testClient2

First off, it would be more consequent to have the same order in both subscriptions since they are very similiar. Secondly, it is a bit strange that the callback is fired 4 times. One would expect two, one for each sub?

Further...

Using a custom PermissionHandler and denying one of the two calls (for example denying subscribing to presence for ALL clients, but allowing presence subscriptions for a specific one) results in the callback for the denied subscription to be fired as well.

See the following code:

const DeepstreamServer = require('deepstream.io');
const DeepstreamClient = require('deepstream.io-client-js');
const EventEmitter = require('events');
// const PermissionHandler = require('../src/permissionhandler');

class PermissionHandler extends EventEmitter {
  constructor() {
    super();
    this.isReady = true;
  }
  canPerformAction(id, request, callback, userData) {
    let result = true;
    if (request.data[0] === 'S') result = false;
    console.log('canPerformAction result:', result);
    callback(null, result);
  }
}

const server = new DeepstreamServer();
server.set('permissionHandler', new PermissionHandler());
server.start();

const subCallback = type => (arg1, arg2) => {
  console.log('subCallback', type, 'arg1:', arg1, 'arg2:', arg2);
};

const client = new DeepstreamClient('localhost:6020');
client.on('error', err => console.log('testClient Error:', err));
client.login({ username: 'testClient', password: 'password' }, success => {
  if (success) {
    client.presence.subscribe('testClient2', subCallback('withId'));
    client.presence.subscribe(subCallback('withoutId'));
    const client2 = new DeepstreamClient('localhost:6020');
    client2.login({ username: 'testClient2', password: 'password' });
  }
});

With output:

INFO | Deepstream started
INCOMING_CONNECTION | from undefined (127.0.0.1)
AUTH_ATTEMPT | 127.0.0.1: AREQ{"username":"testClient","password":"password"}
AUTH_SUCCESSFUL | testClient
canPerformAction result: false
canPerformAction result: true
S | for U:testClient2 by testClient
testClient Error: S
INCOMING_CONNECTION | from undefined (127.0.0.1)
AUTH_ATTEMPT | 127.0.0.1: AREQ{"username":"testClient2","password":"password"}
AUTH_SUCCESSFUL | testClient2
subCallback withoutId arg1: testClient2 arg2: true
subCallback withId arg1: true arg2: testClient2

So, to summarize:

1. Callbacks for presence subscriptions have different argument order when specifying username and not.
2. The docs at https://deepstreamhub.com/docs/client-js/presence/ says the order should be (username, login), which seems most resonable to me.
3. When using both with and without specified name at the same time, too many callbacks are triggered.
4. When denying presence subscriptions for all clients but allowing for specific, the callback is registered for the denied subscription as well.

Thanks!

[yasserf]

Sorry for not addressing this yet. Theres an overarching cleanup for presence but I'm not sure when we will get around to it yet.