diff --git a/js/background.js b/js/background.js index 31620cea59..93858c2c32 100644 --- a/js/background.js +++ b/js/background.js @@ -193,7 +193,7 @@ return; } - return ConversationController.findOrCreateById(id, 'private') + return ConversationController.getOrCreateAndWait(id, 'private') .then(function(conversation) { return new Promise(function(resolve, reject) { conversation.save({ @@ -229,7 +229,7 @@ var details = ev.groupDetails; var id = details.id; - return ConversationController.findOrCreateById(id, 'group').then(function(conversation) { + return ConversationController.getOrCreateAndWait(id, 'group').then(function(conversation) { var updates = { name: details.name, members: details.members, @@ -258,8 +258,19 @@ return; } - return message.handleDataMessage(data.message, ev.confirm, { - initialLoadComplete: initialLoadComplete + var type, id; + if (data.message.group) { + type = 'group'; + id = data.message.group.id; + } else { + type = 'private'; + id = data.source; + } + + return ConversationController.getOrCreateAndWait(id, type).then(function() { + return message.handleDataMessage(data.message, ev.confirm, { + initialLoadComplete: initialLoadComplete + }); }); }); } @@ -286,8 +297,19 @@ return; } - return message.handleDataMessage(data.message, ev.confirm, { - initialLoadComplete: initialLoadComplete + var type, id; + if (data.message.group) { + type = 'group'; + id = data.message.group.id; + } else { + type = 'private'; + id = data.destination; + } + + return ConversationController.getOrCreateAndWait(id, type).then(function() { + return message.handleDataMessage(data.message, ev.confirm, { + initialLoadComplete: initialLoadComplete + }); }); }); } @@ -374,7 +396,7 @@ return message.saveErrors(error).then(function() { var id = message.get('conversationId'); - return ConversationController.findOrCreateById(id, 'private').then(function(conversation) { + return ConversationController.getOrCreateAndWait(id, 'private').then(function(conversation) { conversation.set({ active_at: Date.now(), unreadCount: conversation.get('unreadCount') + 1 @@ -455,7 +477,7 @@ console.log('got verified sync for', number, state, ev.viaContactSync ? 'via contact sync' : ''); - return ConversationController.findOrCreateById(number, 'private').then(function(contact) { + return ConversationController.getOrCreateAndWait(number, 'private').then(function(contact) { var options = { viaSyncMessage: true, viaContactSync: ev.viaContactSync, diff --git a/js/conversation_controller.js b/js/conversation_controller.js index 6972135214..6ac5abda5a 100644 --- a/js/conversation_controller.js +++ b/js/conversation_controller.js @@ -85,37 +85,46 @@ get: function(id) { return conversations.get(id); }, - add: function(attrs) { - return conversations.add(attrs, {merge: true}); + createTemporary: function(attributes) { + return conversations.add(attributes); }, - create: function(attrs) { - if (typeof attrs !== 'object') { - throw new Error('ConversationController.create requires an object, got', attrs); + getOrCreate: function(id, type) { + var conversation = conversations.get(id); + if (conversation) { + return conversation; } - var conversation = conversations.add(attrs, {merge: true}); - return conversation; - }, - findOrCreateById: function(id, type) { - var conversation = conversations.add({ + + conversation = conversations.add({ id: id, type: type }); - return new Promise(function(resolve, reject) { - conversation.fetch().then(function() { + conversation.initialPromise = new Promise(function(resolve, reject) { + var deferred = conversation.save(); + + if (!deferred) { + console.log('Conversation save failed! ', id, type); + return reject(new Error('getOrCreate: Conversation save failed')); + } + + deferred.then(function() { resolve(conversation); - }, function() { - var deferred = conversation.save(); + }, reject); + }); - if (!deferred) { - console.log('Conversation save failed! ', id, type); - return reject(new Error('findOrCreateById: Conversation save failed')); - } + return conversation; + }, + getOrCreateAndWait: function(id, type) { + var conversation = this.getOrCreate(id, type); - deferred.then(function() { - resolve(conversation); - }, reject); + if (conversation) { + return conversation.initialPromise.then(function() { + return conversation; }); - }); + } + + return Promise.reject( + new Error('getOrCreateAndWait: did not get conversation') + ); }, getAllGroupsInvolvingId: function(id) { var groups = new Whisper.GroupCollection(); @@ -126,7 +135,7 @@ }); }, updateInbox: function() { - return conversations.fetchActive(); + return conversations.fetch(); } }; })(); diff --git a/js/keychange_listener.js b/js/keychange_listener.js index b9b550f7cb..927adce9b4 100644 --- a/js/keychange_listener.js +++ b/js/keychange_listener.js @@ -13,13 +13,13 @@ } signalProtocolStore.on('keychange', function(id) { - var conversation = ConversationController.add({id: id}); - conversation.fetch().then(function() { + ConversationController.getOrCreateAndWait(id, 'private').then(function(conversation) { conversation.addKeyChange(id); - }); - ConversationController.getAllGroupsInvolvingId(id).then(function(groups) { - _.forEach(groups, function(group) { - group.addKeyChange(id); + + ConversationController.getAllGroupsInvolvingId(id).then(function(groups) { + _.forEach(groups, function(group) { + group.addKeyChange(id); + }); }); }); }); diff --git a/js/models/conversations.js b/js/models/conversations.js index 303396b8a2..14ac4ba587 100644 --- a/js/models/conversations.js +++ b/js/models/conversations.js @@ -59,6 +59,10 @@ this.ourNumber = textsecure.storage.user.getNumber(); this.verifiedEnum = textsecure.storage.protocol.VerifiedStatus; + // This may be overridden by ConversationController.getOrCreate, and signify + // our first save to the database. Or first fetch from the database. + this.initialPromise = Promise.resolve(); + this.contactCollection = new Backbone.Collection(); this.messageCollection = new Whisper.MessageCollection([], { conversation: this @@ -87,7 +91,7 @@ if (this.isPrivate()) { return Promise.all([ this.safeGetVerified(), - this.safeFetch() + this.initialPromise, ]).then(function(results) { var trust = results[0]; // we don't return here because we don't need to wait for this to finish @@ -103,12 +107,6 @@ }.bind(this)).then(this.onMemberVerifiedChange.bind(this)); } }, - safeFetch: function() { - // new Promise necessary because a fetch will fail if convo not in db yet - return new Promise(function(resolve) { - this.fetch().always(resolve); - }.bind(this)); - }, setVerifiedDefault: function(options) { var DEFAULT = this.verifiedEnum.DEFAULT; return this.queueJob(function() { @@ -801,28 +799,23 @@ return _.contains(this.get('members'), number); }, fetchContacts: function(options) { - return new Promise(function(resolve) { - if (this.isPrivate()) { - this.contactCollection.reset([this]); - resolve(); - } else { - var promises = []; - var members = this.get('members') || []; - - this.contactCollection.reset( - members.map(function(number) { - var c = ConversationController.create({ - id : number, - type : 'private' - }); - this.listenTo(c, 'change:verified', this.onMemberVerifiedChange); - promises.push(c.safeFetch()); - return c; - }.bind(this)) - ); - resolve(Promise.all(promises)); - } - }.bind(this)); + if (this.isPrivate()) { + this.contactCollection.reset([this]); + return Promise.resolve(); + } else { + var members = this.get('members') || []; + var promises = members.map(function(number) { + return ConversationController.getOrCreateAndWait(number, 'private'); + }); + + return Promise.all(promises).then(function(contacts) { + _.forEach(contacts, function(contact) { + this.listenTo(contact, 'change:verified', this.onMemberVerifiedChange); + }.bind(this)); + + this.contactCollection.reset(contacts); + }.bind(this)); + } }, destroyMessages: function() { @@ -956,14 +949,11 @@ } window.drawAttention(); - var sender = ConversationController.create({ - id: message.get('source'), type: 'private' - }); var conversationId = this.id; - return new Promise(function(resolve, reject) { - sender.fetch().then(function() { - sender.getNotificationIcon().then(function(iconUrl) { + ConversationController.getOrCreateAndWait(message.get('source'), 'private') + .then(function(sender) { + return sender.getNotificationIcon().then(function(iconUrl) { console.log('adding notification'); Whisper.Notifications.add({ title : sender.getTitle(), @@ -973,11 +963,8 @@ conversationId : conversationId, messageId : message.id }); - - return resolve(); - }, reject); - }, reject); - }); + }); + }); }, hashCode: function() { if (this.hash === undefined) { @@ -1054,20 +1041,6 @@ } }).always(resolve); }.bind(this)); - }, - - fetchActive: function() { - // Ensures all active conversations are included in this collection, - // and updates their attributes, but removes nothing. - return this.fetch({ - index: { - name: 'inbox', // 'inbox' index on active_at - order: 'desc' // ORDER timestamp DESC - // TODO pagination/infinite scroll - // limit: 10, offset: page*10, - }, - remove: false - }); } }); diff --git a/js/models/messages.js b/js/models/messages.js index 49be47acfa..e47f7e3932 100644 --- a/js/models/messages.js +++ b/js/models/messages.js @@ -147,19 +147,12 @@ return this.imageUrl; }, getConversation: function() { - return ConversationController.add({ - id: this.get('conversationId') - }); + return ConversationController.get(this.get('conversationId')); }, getExpirationTimerUpdateSource: function() { if (this.isExpirationTimerUpdate()) { var conversationId = this.get('expirationTimerUpdate').source; - var c = ConversationController.get(conversationId); - if (!c) { - c = ConversationController.create({id: conversationId, type: 'private'}); - c.fetch(); - } - return c; + return ConversationController.getOrCreate(conversationId, 'private'); } }, getContact: function() { @@ -167,21 +160,12 @@ if (!this.isIncoming()) { conversationId = textsecure.storage.user.getNumber(); } - var c = ConversationController.get(conversationId); - if (!c) { - c = ConversationController.create({id: conversationId, type: 'private'}); - c.fetch(); - } - return c; + return ConversationController.getOrCreate(conversationId, 'private'); }, getModelForKeyChange: function() { var id = this.get('key_changed'); if (!this.modelForKeyChange) { - var c = ConversationController.get(id); - if (!c) { - c = ConversationController.create({ id: id, type: 'private' }); - c.fetch(); - } + var c = ConversationController.getOrCreate(id, 'private'); this.modelForKeyChange = c; } return this.modelForKeyChange; @@ -189,11 +173,7 @@ getModelForVerifiedChange: function() { var id = this.get('verifiedChanged'); if (!this.modelForVerifiedChange) { - var c = ConversationController.get(id); - if (!c) { - c = ConversationController.create({ id: id, type: 'private' }); - c.fetch(); - } + var c = ConversationController.getOrCreate(id, 'private'); this.modelForVerifiedChange = c; } return this.modelForVerifiedChange; @@ -347,9 +327,11 @@ options = options || {}; _.defaults(options, {initialLoadComplete: true}); - // This function can be called from the background script on an - // incoming message or from the frontend after the user accepts an - // identity key change. + // This function is called from the background script in a few scenarios: + // 1. on an incoming message + // 2. on a sent message sync'd from another device + // 3. in rare cases, an incoming message can be retried, though it will + // still through one of the previous two codepaths. var message = this; var source = message.get('source'); var type = message.get('type'); @@ -360,176 +342,174 @@ } console.log('queuing handleDataMessage', message.idForLogging()); - var conversation = ConversationController.create({id: conversationId}); + var conversation = ConversationController.get(conversationId); return conversation.queueJob(function() { return new Promise(function(resolve) { - conversation.fetch().always(function() { - console.log('starting handleDataMessage', message.idForLogging()); - - var now = new Date().getTime(); - var attributes = { type: 'private' }; - if (dataMessage.group) { - var group_update = null; + console.log('starting handleDataMessage', message.idForLogging()); + + var now = new Date().getTime(); + var attributes = { type: 'private' }; + if (dataMessage.group) { + var group_update = null; + attributes = { + type: 'group', + groupId: dataMessage.group.id, + }; + if (dataMessage.group.type === textsecure.protobuf.GroupContext.Type.UPDATE) { attributes = { - type: 'group', - groupId: dataMessage.group.id, + type : 'group', + groupId : dataMessage.group.id, + name : dataMessage.group.name, + avatar : dataMessage.group.avatar, + members : _.union(dataMessage.group.members, conversation.get('members')), }; - if (dataMessage.group.type === textsecure.protobuf.GroupContext.Type.UPDATE) { - attributes = { - type : 'group', - groupId : dataMessage.group.id, - name : dataMessage.group.name, - avatar : dataMessage.group.avatar, - members : _.union(dataMessage.group.members, conversation.get('members')), - }; - group_update = conversation.changedAttributes(_.pick(dataMessage.group, 'name', 'avatar')) || {}; - var difference = _.difference(attributes.members, conversation.get('members')); - if (difference.length > 0) { - group_update.joined = difference; - } - if (conversation.get('left')) { - console.log('re-added to a left group'); - attributes.left = false; - } - } - else if (dataMessage.group.type === textsecure.protobuf.GroupContext.Type.QUIT) { - if (source == textsecure.storage.user.getNumber()) { - attributes.left = true; - group_update = { left: "You" }; - } else { - group_update = { left: source }; - } - attributes.members = _.without(conversation.get('members'), source); + group_update = conversation.changedAttributes(_.pick(dataMessage.group, 'name', 'avatar')) || {}; + var difference = _.difference(attributes.members, conversation.get('members')); + if (difference.length > 0) { + group_update.joined = difference; } - - if (group_update !== null) { - message.set({group_update: group_update}); + if (conversation.get('left')) { + console.log('re-added to a left group'); + attributes.left = false; } } - message.set({ - body : dataMessage.body, - conversationId : conversation.id, - attachments : dataMessage.attachments, - decrypted_at : now, - flags : dataMessage.flags, - errors : [] - }); - if (type === 'outgoing') { - var receipts = Whisper.DeliveryReceipts.forMessage(conversation, message); - receipts.forEach(function(receipt) { - message.set({ - delivered: (message.get('delivered') || 0) + 1 - }); - }); + else if (dataMessage.group.type === textsecure.protobuf.GroupContext.Type.QUIT) { + if (source == textsecure.storage.user.getNumber()) { + attributes.left = true; + group_update = { left: "You" }; + } else { + group_update = { left: source }; + } + attributes.members = _.without(conversation.get('members'), source); } - attributes.active_at = now; - conversation.set(attributes); - if (message.isExpirationTimerUpdate()) { + if (group_update !== null) { + message.set({group_update: group_update}); + } + } + message.set({ + body : dataMessage.body, + conversationId : conversation.id, + attachments : dataMessage.attachments, + decrypted_at : now, + flags : dataMessage.flags, + errors : [] + }); + if (type === 'outgoing') { + var receipts = Whisper.DeliveryReceipts.forMessage(conversation, message); + receipts.forEach(function(receipt) { message.set({ - expirationTimerUpdate: { - source : source, - expireTimer : dataMessage.expireTimer - } + delivered: (message.get('delivered') || 0) + 1 }); - conversation.set({expireTimer: dataMessage.expireTimer}); - } else if (dataMessage.expireTimer) { - message.set({expireTimer: dataMessage.expireTimer}); - } + }); + } + attributes.active_at = now; + conversation.set(attributes); - if (!message.isEndSession() && !message.isGroupUpdate()) { - if (dataMessage.expireTimer) { - if (dataMessage.expireTimer !== conversation.get('expireTimer')) { - conversation.updateExpirationTimer( - dataMessage.expireTimer, source, - message.get('received_at')); - } - } else if (conversation.get('expireTimer')) { - conversation.updateExpirationTimer(null, source, - message.get('received_at')); + if (message.isExpirationTimerUpdate()) { + message.set({ + expirationTimerUpdate: { + source : source, + expireTimer : dataMessage.expireTimer } - } - if (type === 'incoming') { - var readReceipt = Whisper.ReadReceipts.forMessage(message); - if (readReceipt) { - if (message.get('expireTimer') && !message.get('expirationStartTimestamp')) { - message.set('expirationStartTimestamp', readReceipt.get('read_at')); - } + }); + conversation.set({expireTimer: dataMessage.expireTimer}); + } else if (dataMessage.expireTimer) { + message.set({expireTimer: dataMessage.expireTimer}); + } + + if (!message.isEndSession() && !message.isGroupUpdate()) { + if (dataMessage.expireTimer) { + if (dataMessage.expireTimer !== conversation.get('expireTimer')) { + conversation.updateExpirationTimer( + dataMessage.expireTimer, source, + message.get('received_at')); } - if (readReceipt || message.isExpirationTimerUpdate()) { - message.unset('unread'); - // This is primarily to allow the conversation to mark all older messages as - // read, as is done when we receive a read receipt for a message we already - // know about. - Whisper.ReadReceipts.notifyConversation(message); - } else { - conversation.set('unreadCount', conversation.get('unreadCount') + 1); + } else if (conversation.get('expireTimer')) { + conversation.updateExpirationTimer(null, source, + message.get('received_at')); + } + } + if (type === 'incoming') { + var readReceipt = Whisper.ReadReceipts.forMessage(message); + if (readReceipt) { + if (message.get('expireTimer') && !message.get('expirationStartTimestamp')) { + message.set('expirationStartTimestamp', readReceipt.get('read_at')); } } - - var conversation_timestamp = conversation.get('timestamp'); - if (!conversation_timestamp || message.get('sent_at') > conversation_timestamp) { - conversation.set({ - lastMessage : message.getNotificationText(), - timestamp: message.get('sent_at') - }); + if (readReceipt || message.isExpirationTimerUpdate()) { + message.unset('unread'); + // This is primarily to allow the conversation to mark all older messages as + // read, as is done when we receive a read receipt for a message we already + // know about. + Whisper.ReadReceipts.notifyConversation(message); + } else { + conversation.set('unreadCount', conversation.get('unreadCount') + 1); } + } - console.log('beginning saves in handleDataMessage', message.idForLogging()); + var conversation_timestamp = conversation.get('timestamp'); + if (!conversation_timestamp || message.get('sent_at') > conversation_timestamp) { + conversation.set({ + lastMessage : message.getNotificationText(), + timestamp: message.get('sent_at') + }); + } - var handleError = function(error) { - error = error && error.stack ? error.stack : error; - console.log('handleDataMessage', message.idForLogging(), 'error:', error); - return resolve(); - }; + console.log('beginning saves in handleDataMessage', message.idForLogging()); - message.save().then(function() { - conversation.save().then(function() { + var handleError = function(error) { + error = error && error.stack ? error.stack : error; + console.log('handleDataMessage', message.idForLogging(), 'error:', error); + return resolve(); + }; + + message.save().then(function() { + conversation.save().then(function() { + try { + conversation.trigger('newmessage', message); + } + catch (e) { + return handleError(e); + } + // We fetch() here because, between the message.save() above and the previous + // line's trigger() call, we might have marked all messages unread in the + // database. This message might already be read! + var previousUnread = message.get('unread'); + message.fetch().then(function() { try { - conversation.trigger('newmessage', message); + if (previousUnread !== message.get('unread')) { + console.log('Caught race condition on new message read state! ' + + 'Manually starting timers.'); + // We call markRead() even though the message is already marked read + // because we need to start expiration timers, etc. + message.markRead(); + } + if (message.get('unread') && options.initialLoadComplete) { + conversation.notify(message); + } + + console.log('done with handleDataMessage', message.idForLogging()); + + confirm(); + return resolve(); } catch (e) { - return handleError(e); + handleError(e); } - // We fetch() here because, between the message.save() above and the previous - // line's trigger() call, we might have marked all messages unread in the - // database. This message might already be read! - var previousUnread = message.get('unread'); - message.fetch().then(function() { - try { - if (previousUnread !== message.get('unread')) { - console.log('Caught race condition on new message read state! ' + - 'Manually starting timers.'); - // We call markRead() even though the message is already marked read - // because we need to start expiration timers, etc. - message.markRead(); - } - if (message.get('unread') && options.initialLoadComplete) { - conversation.notify(message); - } - - console.log('done with handleDataMessage', message.idForLogging()); - - confirm(); - return resolve(); - } - catch (e) { - handleError(e); - } - }, function(error) { - try { - console.log('handleDataMessage: Message', message.idForLogging(), 'was deleted'); + }, function(error) { + try { + console.log('handleDataMessage: Message', message.idForLogging(), 'was deleted'); - confirm(); - return resolve(); - } - catch (e) { - handleError(e); - } - }); - }, handleError); + confirm(); + return resolve(); + } + catch (e) { + handleError(e); + } + }); }, handleError); - }); + }, handleError); }); }); }, diff --git a/js/notifications.js b/js/notifications.js index 12961e88b2..0b9787e521 100644 --- a/js/notifications.js +++ b/js/notifications.js @@ -25,9 +25,7 @@ openInbox(); return; } - var conversation = ConversationController.create({ - id: last.get('conversationId') - }); + var conversation = ConversationController.get(last.get('conversationId')); openConversation(conversation); this.clear(); }, diff --git a/js/views/conversation_search_view.js b/js/views/conversation_search_view.js index 765baba52d..5c5c45dd64 100644 --- a/js/views/conversation_search_view.js +++ b/js/views/conversation_search_view.js @@ -88,7 +88,7 @@ // Creates a view to display a new contact this.new_contact_view = new Whisper.NewContactView({ el: this.$new_contact, - model: ConversationController.create({ + model: ConversationController.createTemporary({ type: 'private' }) }).render(); @@ -97,7 +97,7 @@ createConversation: function() { var conversation = this.new_contact_view.model; if (this.new_contact_view.model.isValid()) { - ConversationController.findOrCreateById( + ConversationController.getOrCreateAndWait( this.new_contact_view.model.id, 'private' ).then(function(conversation) { diff --git a/js/views/inbox_view.js b/js/views/inbox_view.js index 90fa3f54f0..e29ff7a470 100644 --- a/js/views/inbox_view.js +++ b/js/views/inbox_view.js @@ -261,9 +261,11 @@ }, openConversation: function(e, conversation) { this.searchView.hideHints(); - conversation = ConversationController.create(conversation); - this.conversation_stack.open(conversation); - this.focusConversation(); + if (conversation) { + conversation = ConversationController.get(conversation.id); + this.conversation_stack.open(conversation); + this.focusConversation(); + } }, toggleMenu: function() { this.$('.global-menu .menu-list').toggle(); diff --git a/test/keychange_listener_test.js b/test/keychange_listener_test.js index 1d808b12a1..5db581a945 100644 --- a/test/keychange_listener_test.js +++ b/test/keychange_listener_test.js @@ -26,7 +26,7 @@ describe('KeyChangeListener', function() { describe('When we have a conversation with this contact', function() { var convo = new Whisper.Conversation({ id: phoneNumberWithKeyChange, type: 'private'}); before(function() { - ConversationController.add(convo); + ConversationController.createTemporary(convo); return convo.save(); }); @@ -51,7 +51,7 @@ describe('KeyChangeListener', function() { describe('When we have a group with this contact', function() { var convo = new Whisper.Conversation({ id: 'groupId', type: 'group', members: [phoneNumberWithKeyChange] }); before(function() { - ConversationController.add(convo); + ConversationController.createTemporary(convo); return convo.save(); }); after(function() { diff --git a/test/views/message_view_test.js b/test/views/message_view_test.js index 4cfeb3f798..a9c680603c 100644 --- a/test/views/message_view_test.js +++ b/test/views/message_view_test.js @@ -1,11 +1,9 @@ describe('MessageView', function() { - var conversations = new Whisper.ConversationCollection(); - before(function(done) { - conversations.fetch().then(done); - storage.put('number_id', '+18088888888.1'); + before(function() { + return storage.put('number_id', '+18088888888.1'); }); - var convo = conversations.add({id: 'foo'}); + var convo = ConversationController.createTemporary({id: 'foo'}); var message = convo.messageCollection.add({ conversationId: convo.id, body: 'hello world',