Remove the active flag for devices.

Instead of setting `active` to `false`, we remove the device entirely
(unless its the current device).

Doing it this way means more fetching of bundles for devices that
disappear and then reappear from a user's devicelist.

However, there might be caching invalidation concerns with just reusing
a cached bundle for a device id that disappeared and then reappears.

Additionally this change simplifies the showing of a contact's device
fingerprints in the modal, since we don't have to take active/inactive
into consideration.

updates #497
This commit is contained in:
JC Brand 2018-08-23 10:07:51 +02:00
parent 781ad3d643
commit fd639e2da6
2 changed files with 23 additions and 28 deletions

View File

@ -351,8 +351,6 @@
let devices = _converse.devicelists.get(contact_jid).devices;
expect(devices.length).toBe(2);
expect(_.map(devices.models, 'attributes.id').sort().join()).toBe('1234,4223');
expect(devices.get('1234').get('active')).toBe(true);
expect(devices.get('4223').get('active')).toBe(true);
stanza = $msg({
'from': contact_jid,
@ -368,11 +366,8 @@
_converse.connection._dataRecv(test_utils.createRequest(stanza));
expect(_converse.devicelists.length).toBe(2);
expect(devices.length).toBe(3);
expect(_.map(devices.models, 'attributes.id').sort().join()).toBe('1234,4223,4224');
expect(devices.get('1234').get('active')).toBe(false);
expect(devices.get('4223').get('active')).toBe(true);
expect(devices.get('4224').get('active')).toBe(true);
expect(devices.length).toBe(2);
expect(_.map(devices.models, 'attributes.id').sort().join()).toBe('4223,4224');
// Check that own devicelist gets updated
stanza = $msg({
@ -393,9 +388,6 @@
devices = _converse.devicelists.get(_converse.bare_jid).devices;
expect(devices.length).toBe(3);
expect(_.map(devices.models, 'attributes.id').sort().join()).toBe('123456789,555,777');
expect(devices.get('123456789').get('active')).toBe(true);
expect(devices.get('555').get('active')).toBe(true);
expect(devices.get('777').get('active')).toBe(true);
_converse.connection.IQ_stanzas = [];
@ -441,13 +433,9 @@
expect(_converse.devicelists.length).toBe(2);
const devices = _converse.devicelists.get(_converse.bare_jid).devices;
// The device id for this device (123456789) was also generated and added to the list,
// which is why we have 4 devices now.
expect(devices.length).toBe(4);
expect(_.map(devices.models, 'attributes.id').sort().join()).toBe('123456789,444,555,777');
expect(devices.get('123456789').get('active')).toBe(true);
expect(devices.get('444').get('active')).toBe(true);
expect(devices.get('555').get('active')).toBe(false);
expect(devices.get('777').get('active')).toBe(false);
// which is why we have 2 devices now.
expect(devices.length).toBe(2);
expect(_.map(devices.models, 'attributes.id').sort().join()).toBe('123456789,444');
done();
}).catch(_.partial(_converse.log, _, Strophe.LogLevel.FATAL))
}));
@ -994,6 +982,9 @@
expect(modal.el.querySelectorAll('input[type="radio"]').length).toBe(2);
const devicelist = _converse.devicelists.get(contact_jid);
expect(devicelist.devices.get('555').get('trusted')).toBe(0);
let trusted_radio = modal.el.querySelector('input[type="radio"][name="555"][value="1"]');
expect(trusted_radio.checked).toBe(true);
@ -1004,9 +995,13 @@
untrusted_radio.click();
trusted_radio = document.querySelector('input[type="radio"][name="555"][value="1"]');
expect(trusted_radio.hasAttribute('checked')).toBe(false);
expect(devicelist.devices.get('555').get('trusted')).toBe(-1);
untrusted_radio = document.querySelector('input[type="radio"][name="555"][value="-1"]');
expect(untrusted_radio.hasAttribute('checked')).toBe(true);
trusted_radio.click();
expect(devicelist.devices.get('555').get('trusted')).toBe(1);
done();
}).catch(_.partial(_converse.log, _, Strophe.LogLevel.FATAL))
}));

View File

@ -672,7 +672,6 @@
_converse.Device = Backbone.Model.extend({
defaults: {
'active': true,
'trusted': UNDECIDED
},
@ -758,8 +757,7 @@
const device_id = _converse.omemo_store.get('device_id'),
own_device = this.devices.findWhere({'id': device_id});
if (!_.includes(device_ids, device_id) || !own_device.get('active')) {
own_device.save('active', true, {'silent': true});
if (!_.includes(device_ids, device_id)) {
return this.publishDevices();
}
});
@ -788,9 +786,7 @@
.c('publish', {'node': Strophe.NS.OMEMO_DEVICELIST})
.c('item')
.c('list', {'xmlns': Strophe.NS.OMEMO})
_.each(this.devices.where({'active': true}), (device) => {
stanza.c('device', {'id': device.get('id')}).up();
});
this.devices.each(device => stanza.c('device', {'id': device.get('id')}).up());
return _converse.api.sendIQ(stanza);
},
@ -881,12 +877,16 @@
devices = devicelist.devices,
removed_ids = _.difference(devices.pluck('id'), device_ids);
_.forEach(removed_ids, (removed_id) => devices.get(removed_id).save('active', false));
_.forEach(removed_ids, (id) => {
if (jid === _converse.bare_jid && id === _converse.omemo_store.get('device_id')) {
// We don't remove the current device
return
}
devices.get(id).destroy();
});
_.forEach(device_ids, (device_id) => {
const dev = devices.get(device_id);
if (dev) {
dev.save({'active': true});
} else {
if (!devices.get(device_id)) {
devices.create({'id': device_id, 'jid': jid})
}
});