Fix race condition where waitUntilFeaturesDiscovered was resolved too early

When calling `api.supports(feature, entity_jid)`, it checks whether the entity
supports the feature or whether any of the sub-items on the entity
supports that feature.

However, on `DiscoEntity`, the `waitUntilFeaturesDiscovered` promise
didn't wait for the items on the entity to be fetched, and was therefore
resolved too quickly.

This caused the file upload button to not render.

Updates #2925
This commit is contained in:
JC Brand 2022-11-08 18:48:38 +01:00
parent 3af6168270
commit a251608fc5
4 changed files with 45 additions and 50 deletions

View File

@ -159,7 +159,7 @@ const DiscoEntity = Model.extend({
this.onDiscoItems(stanza);
},
onInfo (stanza) {
async onInfo (stanza) {
Array.from(stanza.querySelectorAll('identity')).forEach(identity => {
this.identities.create({
'category': identity.getAttribute('category'),
@ -180,7 +180,7 @@ const DiscoEntity = Model.extend({
});
if (stanza.querySelector(`feature[var="${Strophe.NS.DISCO_ITEMS}"]`)) {
this.queryForItems();
await this.queryForItems();
}
Array.from(stanza.querySelectorAll('feature')).forEach(feature => {
this.features.create({

View File

@ -2,7 +2,7 @@
describe("Service Discovery", function () {
describe("Whenever converse.js queries a server for its features", function () {
describe("Whenever a server is queried for its features", function () {
it("stores the features it receives",
mock.initConverse(
@ -76,23 +76,12 @@ describe("Service Discovery", function () {
'var': 'jabber:iq:version'});
_converse.connection._dataRecv(mock.createRequest(stanza));
let entities = await _converse.api.disco.entities.get()
expect(entities.length).toBe(2); // We have an extra entity, which is the user's JID
expect(entities.get(_converse.domain).features.length).toBe(5);
expect(entities.get(_converse.domain).identities.length).toBe(3);
expect(entities.get('montague.lit').features.where({'var': 'jabber:iq:version'}).length).toBe(1);
expect(entities.get('montague.lit').features.where({'var': 'jabber:iq:time'}).length).toBe(1);
expect(entities.get('montague.lit').features.where({'var': 'jabber:iq:register'}).length).toBe(1);
expect(entities.get('montague.lit').features.where(
{'var': 'http://jabber.org/protocol/disco#items'}).length).toBe(1);
expect(entities.get('montague.lit').features.where(
{'var': 'http://jabber.org/protocol/disco#info'}).length).toBe(1);
await u.waitUntil(function () {
// Converse.js sees that the entity has a disco#items feature,
// so it will make a query for it.
return IQ_stanzas.filter(iq => iq.querySelector('query[xmlns="http://jabber.org/protocol/disco#items"]')).length > 0;
});
/* <iq type='result'
* from='catalog.shakespeare.lit'
* to='romeo@montague.net/orchard'
@ -119,9 +108,8 @@ describe("Service Discovery", function () {
* </query>
* </iq>
*/
stanza = IQ_stanzas.find(function (iq) {
return iq.querySelector('iq[to="montague.lit"] query[xmlns="http://jabber.org/protocol/disco#items"]');
});
stanza = IQ_stanzas.find(iq => iq.querySelector('iq[to="montague.lit"] query[xmlns="http://jabber.org/protocol/disco#items"]'));
const items_IQ_id = IQ_ids[IQ_stanzas.indexOf(stanza)];
stanza = $iq({
'type': 'result',
@ -152,9 +140,19 @@ describe("Service Discovery", function () {
});
_converse.connection._dataRecv(mock.createRequest(stanza));
await u.waitUntil(() => _converse.disco_entities);
entities = _converse.disco_entities;
expect(entities.length).toBe(5);
const entities = await _converse.api.disco.entities.get()
expect(entities.length).toBe(5); // We have an extra entity, which is the user's JID
expect(entities.get(_converse.domain).features.length).toBe(5);
expect(entities.get(_converse.domain).identities.length).toBe(3);
expect(entities.get('montague.lit').features.where({'var': 'jabber:iq:version'}).length).toBe(1);
expect(entities.get('montague.lit').features.where({'var': 'jabber:iq:time'}).length).toBe(1);
expect(entities.get('montague.lit').features.where({'var': 'jabber:iq:register'}).length).toBe(1);
expect(entities.get('montague.lit').features.where(
{'var': 'http://jabber.org/protocol/disco#items'}).length).toBe(1);
expect(entities.get('montague.lit').features.where(
{'var': 'http://jabber.org/protocol/disco#info'}).length).toBe(1);
expect(entities.map(e => e.get('jid'))).toEqual([
'montague.lit',
'romeo@montague.lit',

View File

@ -42,13 +42,6 @@ describe("XEP-0363: HTTP File Upload", function () {
'var': 'http://jabber.org/protocol/disco#items'});
_converse.connection._dataRecv(mock.createRequest(stanza));
let entities = await _converse.api.disco.entities.get();
expect(entities.length).toBe(2);
expect(entities.pluck('jid')).toEqual(['montague.lit', 'romeo@montague.lit']);
expect(entities.get(_converse.domain).features.length).toBe(2);
expect(entities.get(_converse.domain).identities.length).toBe(1);
// Converse.js sees that the entity has a disco#items feature,
// so it will make a query for it.
selector = 'iq[to="montague.lit"] query[xmlns="http://jabber.org/protocol/disco#items"]';
@ -77,6 +70,13 @@ describe("XEP-0363: HTTP File Upload", function () {
_converse.connection._dataRecv(mock.createRequest(stanza));
let entities = await _converse.api.disco.entities.get();
expect(entities.length).toBe(3);
expect(entities.pluck('jid')).toEqual(['montague.lit', 'romeo@montague.lit', 'upload.montague.lit']);
expect(entities.get(_converse.domain).features.length).toBe(2);
expect(entities.get(_converse.domain).identities.length).toBe(1);
_converse.api.disco.entities.get().then(entities => {
expect(entities.length).toBe(3);
expect(entities.pluck('jid')).toEqual(['montague.lit', 'romeo@montague.lit', 'upload.montague.lit']);
@ -162,7 +162,12 @@ describe("XEP-0363: HTTP File Upload", function () {
describe("A file upload toolbar button", function () {
it("appears in private chats", mock.initConverse(async (_converse) => {
it("appears in private chats", mock.initConverse([], {}, async (_converse) => {
await mock.waitForRoster(_converse, 'current', 3);
const contact_jid = mock.cur_names[2].replace(/ /g,'.').toLowerCase() + '@montague.lit';
await mock.openChatBoxFor(_converse, contact_jid);
const view = _converse.chatboxviews.get(contact_jid);
await mock.waitUntilDiscoConfirmed(
_converse, _converse.domain,
[{'category': 'server', 'type':'IM'}],
@ -170,10 +175,7 @@ describe("XEP-0363: HTTP File Upload", function () {
await mock.waitUntilDiscoConfirmed(_converse, _converse.domain, [], [], ['upload.montague.lit'], 'items')
await mock.waitUntilDiscoConfirmed(_converse, 'upload.montague.lit', [], [Strophe.NS.HTTPUPLOAD], []);
await mock.waitForRoster(_converse, 'current', 3);
const contact_jid = mock.cur_names[2].replace(/ /g,'.').toLowerCase() + '@montague.lit';
await mock.openChatBoxFor(_converse, contact_jid);
const view = _converse.chatboxviews.get(contact_jid);
const el = await u.waitUntil(() => view.querySelector('.chat-toolbar .fileupload'));
expect(el).not.toEqual(null);
}));
@ -307,14 +309,6 @@ describe("XEP-0363: HTTP File Upload", function () {
.c('feature', {
'var': 'http://jabber.org/protocol/disco#items'});
_converse.connection._dataRecv(mock.createRequest(stanza));
let entities = await _converse.api.disco.entities.get();
expect(entities.length).toBe(2);
expect(entities.pluck('jid').includes('montague.lit')).toBe(true);
expect(entities.pluck('jid').includes('romeo@montague.lit')).toBe(true);
expect(entities.get(_converse.domain).features.length).toBe(2);
expect(entities.get(_converse.domain).identities.length).toBe(1);
await u.waitUntil(function () {
// Converse.js sees that the entity has a disco#items feature,
@ -327,7 +321,7 @@ describe("XEP-0363: HTTP File Upload", function () {
stanza = IQ_stanzas.find(function (iq) {
return iq.querySelector('iq[to="montague.lit"] query[xmlns="http://jabber.org/protocol/disco#items"]');
});
var items_IQ_id = IQ_ids[IQ_stanzas.indexOf(stanza)];
const items_IQ_id = IQ_ids[IQ_stanzas.indexOf(stanza)];
stanza = $iq({
'type': 'result',
'from': 'montague.lit',
@ -340,9 +334,11 @@ describe("XEP-0363: HTTP File Upload", function () {
_converse.connection._dataRecv(mock.createRequest(stanza));
entities = await _converse.api.disco.entities.get()
let entities = await _converse.api.disco.entities.get();
expect(entities.length).toBe(3);
expect(entities.get(_converse.domain).features.length).toBe(2);
expect(entities.get(_converse.domain).identities.length).toBe(1);
expect(entities.pluck('jid')).toEqual(['montague.lit', 'romeo@montague.lit', 'upload.montague.lit']);
expect(entities.get('montague.lit').items.length).toBe(1);
await u.waitUntil(function () {

View File

@ -28,13 +28,6 @@ describe("Service Discovery", function () {
.c('feature', { 'var': 'http://jabber.org/protocol/disco#items'}).up();
_converse.connection._dataRecv(mock.createRequest(stanza));
const entities = await _converse.api.disco.entities.get();
expect(entities.length).toBe(2); // We have an extra entity, which is the user's JID
expect(entities.get(_converse.domain).identities.length).toBe(2);
expect(entities.get('montague.lit').features.where(
{'var': 'http://jabber.org/protocol/disco#items'}).length).toBe(1);
expect(entities.get('montague.lit').features.where(
{'var': 'http://jabber.org/protocol/disco#info'}).length).toBe(1);
stanza = await u.waitUntil(() => IQ_stanzas.filter(
iq => iq.querySelector('iq[to="montague.lit"] query[xmlns="http://jabber.org/protocol/disco#items"]')).pop()
@ -60,6 +53,14 @@ describe("Service Discovery", function () {
.c('identity', { 'category': 'conference', 'name': 'Play-Specific Chatrooms', 'type': 'text'}).up()
.c('feature', { 'var': 'http://jabber.org/protocol/muc'}).up()));
const entities = await _converse.api.disco.entities.get();
expect(entities.length).toBe(3); // We have an extra entity, which is the user's JID
expect(entities.get(_converse.domain).identities.length).toBe(2);
expect(entities.get('montague.lit').features.where(
{'var': 'http://jabber.org/protocol/disco#items'}).length).toBe(1);
expect(entities.get('montague.lit').features.where(
{'var': 'http://jabber.org/protocol/disco#info'}).length).toBe(1);
await u.waitUntil(() => _converse.api.settings.get('muc_domain') === 'chat.shakespeare.lit');
}));
});