From 8ef9c8ed1ae3040c157c6a9e5fe69df4ace24c2c Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Tue, 15 Jun 2021 10:12:43 +1000 Subject: [PATCH] remove delivery receipt logic --- background.html | 1 - background_test.html | 1 - js/background.js | 9 -- js/delivery_receipts.js | 104 ------------------ protos/SignalService.proto | 1 - test/fixtures.js | 1 - ts/components/conversation/ExpireTimer.tsx | 3 +- .../conversation/message/MessageMetadata.tsx | 8 +- .../message/OutgoingMessageStatus.tsx | 18 +-- ts/models/message.ts | 9 -- ts/models/messageType.ts | 23 ++-- ts/receiver/contentMessage.ts | 21 +--- ts/receiver/dataMessage.ts | 16 --- ts/receiver/queuedJob.ts | 22 +--- .../receipt/DeliveryReceiptMessage.ts | 8 -- ts/state/ducks/conversations.ts | 3 +- .../unit/messages/ReceiptMessage_test.ts | 13 --- 17 files changed, 25 insertions(+), 236 deletions(-) delete mode 100644 js/delivery_receipts.js delete mode 100644 ts/session/messages/outgoing/controlMessage/receipt/DeliveryReceiptMessage.ts diff --git a/background.html b/background.html index 58cf594e8..53b8e74f0 100644 --- a/background.html +++ b/background.html @@ -118,7 +118,6 @@ - diff --git a/background_test.html b/background_test.html index b06443315..c2a12e3f9 100644 --- a/background_test.html +++ b/background_test.html @@ -123,7 +123,6 @@ - diff --git a/js/background.js b/js/background.js index a35b84626..911772db6 100644 --- a/js/background.js +++ b/js/background.js @@ -259,21 +259,12 @@ window.log.info(`Cleanup: Found ${messagesForCleanup.length} messages for cleanup`); await Promise.all( messagesForCleanup.map(async message => { - const delivered = message.get('delivered'); const sentAt = message.get('sent_at'); - const expirationStartTimestamp = message.get('expirationStartTimestamp'); if (message.hasErrors()) { return; } - if (delivered) { - window.log.info(`Cleanup: Starting timer for delivered message ${sentAt}`); - message.set('expirationStartTimestamp', expirationStartTimestamp || sentAt); - await message.setToExpire(); - return; - } - window.log.info(`Cleanup: Deleting unsent message ${sentAt}`); await window.Signal.Data.removeMessage(message.id); }) diff --git a/js/delivery_receipts.js b/js/delivery_receipts.js deleted file mode 100644 index d24a49207..000000000 --- a/js/delivery_receipts.js +++ /dev/null @@ -1,104 +0,0 @@ -/* global - Backbone, - Whisper, - getMessageController, - _, -*/ - -/* eslint-disable more/no-then */ - -// eslint-disable-next-line func-names -(function() { - 'use strict'; - - window.Whisper = window.Whisper || {}; - - Whisper.DeliveryReceipts = new (Backbone.Collection.extend({ - forMessage(conversation, message) { - let recipients; - if (conversation.isPrivate()) { - recipients = [conversation.id]; - } else { - recipients = conversation.get('members') || []; - } - const receipts = this.filter( - receipt => - receipt.get('timestamp') === message.get('sent_at') && - recipients.indexOf(receipt.get('source')) > -1 - ); - this.remove(receipts); - return receipts; - }, - async getTargetMessage(originalSource, messages) { - if (messages.length === 0) { - return null; - } - - const message = messages.find( - item => !item.isIncoming() && originalSource === item.get('conversationId') - ); - if (message) { - return message; - } - - const groups = await window.Signal.Data.getAllGroupsInvolvingId(originalSource); - - const ids = groups.pluck('id'); - ids.push(originalSource); - - const target = messages.find( - item => !item.isIncoming() && _.contains(ids, item.get('conversationId')) - ); - if (!target) { - return null; - } - - return getMessageController().register(target.id, target); - }, - async onReceipt(receipt) { - try { - const messages = await window.Signal.Data.getMessagesBySentAt(receipt.get('timestamp')); - - const message = await this.getTargetMessage(receipt.get('source'), messages); - if (!message) { - window.log.info( - 'No message for delivery receipt', - receipt.get('source'), - receipt.get('timestamp') - ); - return; - } - - const deliveries = message.get('delivered') || 0; - const deliveredTo = message.get('delivered_to') || []; - const expirationStartTimestamp = message.get('expirationStartTimestamp'); - message.set({ - delivered_to: _.union(deliveredTo, [receipt.get('source')]), - delivered: deliveries + 1, - expirationStartTimestamp: expirationStartTimestamp || Date.now(), - sent: true, - }); - - if (message.isExpiring() && !expirationStartTimestamp) { - // This will save the message for us while starting the timer - await message.setToExpire(); - } else { - await message.commit(); - } - - // notify frontend listeners - const conversation = window.getConversationController().get(message.get('conversationId')); - if (conversation) { - conversation.updateLastMessage(); - } - - this.remove(receipt); - } catch (error) { - window.log.error( - 'DeliveryReceipts.onReceipt error:', - error && error.stack ? error.stack : error - ); - } - }, - }))(); -})(); diff --git a/protos/SignalService.proto b/protos/SignalService.proto index ac8536809..69347e53e 100644 --- a/protos/SignalService.proto +++ b/protos/SignalService.proto @@ -176,7 +176,6 @@ message ConfigurationMessage { message ReceiptMessage { enum Type { - DELIVERY = 0; READ = 1; } diff --git a/test/fixtures.js b/test/fixtures.js index c02df8144..28bf135e5 100644 --- a/test/fixtures.js +++ b/test/fixtures.js @@ -229,7 +229,6 @@ Whisper.Fixtures = () => { type: 'outgoing', body: 'See you all there!', recipients: [MICHEL_ID, FRED_ID, NESTOR_ID], - delivered_to: [MICHEL_ID, FRED_ID], sent_to: [NESTOR_ID], conversationId: group.id, }); diff --git a/ts/components/conversation/ExpireTimer.tsx b/ts/components/conversation/ExpireTimer.tsx index c743310d5..e3ef1819c 100644 --- a/ts/components/conversation/ExpireTimer.tsx +++ b/ts/components/conversation/ExpireTimer.tsx @@ -5,12 +5,13 @@ import { useInterval } from '../../hooks/useInterval'; import styled, { DefaultTheme } from 'styled-components'; import { OpacityMetadataComponent } from './message/MessageMetadata'; import { SessionIcon, SessionIconSize } from '../session/icon'; +import { MessageModelType } from '../../models/messageType'; type Props = { withImageNoCaption: boolean; expirationLength: number; expirationTimestamp: number; - direction: 'incoming' | 'outgoing'; + direction: MessageModelType; theme: DefaultTheme; }; diff --git a/ts/components/conversation/message/MessageMetadata.tsx b/ts/components/conversation/message/MessageMetadata.tsx index c5120a103..6a27b2286 100644 --- a/ts/components/conversation/message/MessageMetadata.tsx +++ b/ts/components/conversation/message/MessageMetadata.tsx @@ -1,13 +1,11 @@ import React from 'react'; -import classNames from 'classnames'; - import { MessageSendingErrorText, MetadataSpacer } from './MetadataUtilComponent'; import { OutgoingMessageStatus } from './OutgoingMessageStatus'; -import { Spinner } from '../../basic/Spinner'; import { MetadataBadges } from './MetadataBadge'; import { Timestamp } from '../Timestamp'; import { ExpireTimer } from '../ExpireTimer'; import styled, { DefaultTheme } from 'styled-components'; +import { MessageDeliveryStatus, MessageModelType } from '../../../models/messageType'; type Props = { disableMenu?: boolean; @@ -16,10 +14,10 @@ type Props = { text?: string; id: string; collapseMetadata?: boolean; - direction: 'incoming' | 'outgoing'; + direction: MessageModelType; timestamp: number; serverTimestamp?: number; - status?: 'sending' | 'sent' | 'delivered' | 'read' | 'error'; + status?: MessageDeliveryStatus; expirationLength?: number; expirationTimestamp?: number; isPublic?: boolean; diff --git a/ts/components/conversation/message/OutgoingMessageStatus.tsx b/ts/components/conversation/message/OutgoingMessageStatus.tsx index 2ad84232c..64f1e6acd 100644 --- a/ts/components/conversation/message/OutgoingMessageStatus.tsx +++ b/ts/components/conversation/message/OutgoingMessageStatus.tsx @@ -1,5 +1,6 @@ import React from 'react'; import styled, { DefaultTheme } from 'styled-components'; +import { MessageDeliveryStatus } from '../../../models/messageType'; import { SessionIcon, SessionIconSize, SessionIconType } from '../../session/icon'; import { OpacityMetadataComponent } from './MessageMetadata'; @@ -36,19 +37,6 @@ const MessageStatusSent = (props: { theme: DefaultTheme; iconColor: string }) => ); }; -const MessageStatusDelivered = (props: { theme: DefaultTheme; iconColor: string }) => { - return ( - - - - ); -}; - const MessageStatusRead = (props: { theme: DefaultTheme; iconColor: string }) => { return ( @@ -76,7 +64,7 @@ const MessageStatusError = (props: { theme: DefaultTheme }) => { }; export const OutgoingMessageStatus = (props: { - status?: 'sending' | 'sent' | 'delivered' | 'read' | 'error'; + status?: MessageDeliveryStatus; theme: DefaultTheme; iconColor: string; isInMessageView?: boolean; @@ -86,8 +74,6 @@ export const OutgoingMessageStatus = (props: { return ; case 'sent': return ; - case 'delivered': - return ; case 'read': return ; case 'error': diff --git a/ts/models/message.ts b/ts/models/message.ts index 00a31d22a..881bd10ac 100644 --- a/ts/models/message.ts +++ b/ts/models/message.ts @@ -463,11 +463,6 @@ export class MessageModel extends Backbone.Model { if (window.storage.get('read-receipt-setting') && readBy.length > 0) { return 'read'; } - const delivered = this.get('delivered'); - const deliveredTo = this.get('delivered_to') || []; - if (delivered || deliveredTo.length > 0) { - return 'delivered'; - } const sent = this.get('sent'); const sentTo = this.get('sent_to') || []; if (sent || sentTo.length > 0) { @@ -978,10 +973,6 @@ export class MessageModel extends Backbone.Model { if (readBy.indexOf(pubkey) >= 0) { return 'read'; } - const deliveredTo = this.get('delivered_to') || []; - if (deliveredTo.indexOf(pubkey) >= 0) { - return 'delivered'; - } const sentTo = this.get('sent_to') || []; if (sentTo.indexOf(pubkey) >= 0) { return 'sent'; diff --git a/ts/models/messageType.ts b/ts/models/messageType.ts index de6dd7184..f176f5107 100644 --- a/ts/models/messageType.ts +++ b/ts/models/messageType.ts @@ -7,7 +7,7 @@ import { Contact } from '../types/Contact'; import { ConversationTypeEnum } from './conversation'; export type MessageModelType = 'incoming' | 'outgoing'; -export type MessageDeliveryStatus = 'sending' | 'sent' | 'delivered' | 'read' | 'error'; +export type MessageDeliveryStatus = 'sending' | 'sent' | 'read' | 'error'; export interface MessageAttributes { // the id of the message @@ -23,11 +23,9 @@ export interface MessageAttributes { body?: string; expirationStartTimestamp: number; read_by: Array; - delivered_to: Array; decrypted_at: number; expires_at?: number; recipients: Array; - delivered?: number; type: MessageModelType; group_update?: any; groupInvitation?: any; @@ -127,11 +125,9 @@ export interface MessageAttributesOptionals { body?: string; expirationStartTimestamp?: number; read_by?: Array; - delivered_to?: Array; decrypted_at?: number; expires_at?: number; recipients?: Array; - delivered?: number; type: MessageModelType; group_update?: any; groupInvitation?: any; @@ -178,13 +174,22 @@ export interface MessageAttributesOptionals { export const fillMessageAttributesWithDefaults = ( optAttributes: MessageAttributesOptionals ): MessageAttributes => { - //FIXME to do put the default - return _.defaults(optAttributes, { + const defaulted = _.defaults(optAttributes, { expireTimer: 0, // disabled id: uuidv4(), schemaVersion: window.Signal.Types.Message.CURRENT_SCHEMA_VERSION, unread: 0, // if nothing is set, this message is considered read }); + // this is just to cleanup a bit the db. delivered and delivered_to were removed, so everytime we load a message + // we make sure to clean those fields in the json. + // the next commit() will write that to the disk + if (defaulted.delivered) { + delete defaulted.delivered; + } + if (defaulted.delivered_to) { + delete defaulted.delivered_to; + } + return defaulted; }; export interface MessageRegularProps { @@ -195,10 +200,10 @@ export interface MessageRegularProps { text?: string; id: string; collapseMetadata?: boolean; - direction: 'incoming' | 'outgoing'; + direction: MessageModelType; timestamp: number; serverTimestamp?: number; - status?: 'sending' | 'sent' | 'delivered' | 'read' | 'error'; + status?: MessageDeliveryStatus; // What if changed this over to a single contact like quote, and put the events on it? contact?: Contact & { onSendMessage?: () => void; diff --git a/ts/receiver/contentMessage.ts b/ts/receiver/contentMessage.ts index a1f669008..fe1ec64e0 100644 --- a/ts/receiver/contentMessage.ts +++ b/ts/receiver/contentMessage.ts @@ -383,20 +383,6 @@ function onReadReceipt(readAt: any, timestamp: any, reader: any) { return Whisper.ReadReceipts.onReceipt(receipt); } -export function onDeliveryReceipt(source: any, timestamp: any) { - const { Whisper } = window; - - window?.log?.info('delivery receipt from', `${source}.${1}`, timestamp); - - const receipt = Whisper.DeliveryReceipts.add({ - timestamp, - source, - }); - - // Calling this directly so we can wait for completion - return Whisper.DeliveryReceipts.onReceipt(receipt); -} - async function handleReceiptMessage( envelope: EnvelopePlus, receiptMessage: SignalService.IReceiptMessage @@ -406,12 +392,7 @@ async function handleReceiptMessage( const { type, timestamp } = receipt; const results = []; - if (type === SignalService.ReceiptMessage.Type.DELIVERY) { - for (const ts of timestamp) { - const promise = onDeliveryReceipt(envelope.source, Lodash.toNumber(ts)); - results.push(promise); - } - } else if (type === SignalService.ReceiptMessage.Type.READ) { + if (type === SignalService.ReceiptMessage.Type.READ) { for (const ts of timestamp) { const promise = onReadReceipt( Lodash.toNumber(envelope.timestamp), diff --git a/ts/receiver/dataMessage.ts b/ts/receiver/dataMessage.ts index c8637ade0..d8f7da9e7 100644 --- a/ts/receiver/dataMessage.ts +++ b/ts/receiver/dataMessage.ts @@ -19,7 +19,6 @@ import { getMessageBySenderAndServerTimestamp, } from '../../ts/data/data'; import { ConversationModel, ConversationTypeEnum } from '../models/conversation'; -import { DeliveryReceiptMessage } from '../session/messages/outgoing/controlMessage/receipt/DeliveryReceiptMessage'; import { allowOnlyOneAtATime } from '../session/utils/Promise'; export async function updateProfileOneAtATime( @@ -553,15 +552,6 @@ export function createMessage(data: MessageCreationData, isIncoming: boolean): M } } -function sendDeliveryReceipt(source: string, timestamp: any) { - const receiptMessage = new DeliveryReceiptMessage({ - timestamp: Date.now(), - timestamps: [timestamp], - }); - const device = new PubKey(source); - void getMessageQueue().sendToPubKey(device, receiptMessage); -} - export interface MessageEvent { data: any; type: string; @@ -605,12 +595,6 @@ export async function handleMessageEvent(event: MessageEvent): Promise { const isOurDevice = UserUtils.isUsFromCache(source); - const shouldSendReceipt = isIncoming && !isGroupMessage && !isOurDevice; - - if (shouldSendReceipt) { - sendDeliveryReceipt(source, data.timestamp); - } - // Conversation Id is: // - primarySource if it is an incoming DM message, // - destination if it is an outgoing message, diff --git a/ts/receiver/queuedJob.ts b/ts/receiver/queuedJob.ts index 7e2f228cd..d2b3341ba 100644 --- a/ts/receiver/queuedJob.ts +++ b/ts/receiver/queuedJob.ts @@ -258,37 +258,17 @@ function handleSyncedReceipts(message: MessageModel, conversation: ConversationM }); } - const deliveryReceipts = window.Whisper.DeliveryReceipts.forMessage(conversation, message); - - if (deliveryReceipts.length) { - handleSyncDeliveryReceipts(message, deliveryReceipts); - } - - // A sync'd message to ourself is automatically considered read and delivered + // A sync'd message to ourself is automatically considered read const recipients = conversation.getRecipients(); if (conversation.isMe()) { message.set({ read_by: recipients, - delivered_to: recipients, }); } message.set({ recipients }); } -function handleSyncDeliveryReceipts(message: MessageModel, receipts: any) { - const sources = receipts.map((receipt: any) => receipt.get('source')); - - const deliveredTo = _.union(message.get('delivered_to') || [], sources); - - const deliveredCount = deliveredTo.length; - - message.set({ - delivered: deliveredCount, - delivered_to: deliveredTo, - }); -} - async function handleRegularMessage( conversation: ConversationModel, message: MessageModel, diff --git a/ts/session/messages/outgoing/controlMessage/receipt/DeliveryReceiptMessage.ts b/ts/session/messages/outgoing/controlMessage/receipt/DeliveryReceiptMessage.ts deleted file mode 100644 index 5dcef5ff0..000000000 --- a/ts/session/messages/outgoing/controlMessage/receipt/DeliveryReceiptMessage.ts +++ /dev/null @@ -1,8 +0,0 @@ -import { SignalService } from '../../../../../protobuf'; -import { ReceiptMessage } from './ReceiptMessage'; - -export class DeliveryReceiptMessage extends ReceiptMessage { - public getReceiptType(): SignalService.ReceiptMessage.Type { - return SignalService.ReceiptMessage.Type.DELIVERY; - } -} diff --git a/ts/state/ducks/conversations.ts b/ts/state/ducks/conversations.ts index f5bcbd78d..2b89103fc 100644 --- a/ts/state/ducks/conversations.ts +++ b/ts/state/ducks/conversations.ts @@ -6,6 +6,7 @@ import { ConversationController } from '../../session/conversations'; import { MessageModel } from '../../models/message'; import { getMessagesByConversation } from '../../data/data'; import { ConversationTypeEnum } from '../../models/conversation'; +import { MessageDeliveryStatus } from '../../models/messageType'; // State @@ -50,7 +51,7 @@ export type MessageTypeInConvo = { getPropsForMessageDetail(): Promise; }; -export type LastMessageStatusType = 'error' | 'sending' | 'sent' | 'delivered' | 'read' | null; +export type LastMessageStatusType = MessageDeliveryStatus | null; export type LastMessageType = { status: LastMessageStatusType; diff --git a/ts/test/session/unit/messages/ReceiptMessage_test.ts b/ts/test/session/unit/messages/ReceiptMessage_test.ts index 94af6e3b7..e4c479be3 100644 --- a/ts/test/session/unit/messages/ReceiptMessage_test.ts +++ b/ts/test/session/unit/messages/ReceiptMessage_test.ts @@ -4,19 +4,16 @@ import { beforeEach } from 'mocha'; import { SignalService } from '../../../../protobuf'; import { toNumber } from 'lodash'; import { Constants } from '../../../../session'; -import { DeliveryReceiptMessage } from '../../../../session/messages/outgoing/controlMessage/receipt/DeliveryReceiptMessage'; import { ReadReceiptMessage } from '../../../../session/messages/outgoing/controlMessage/receipt/ReadReceiptMessage'; describe('ReceiptMessage', () => { let readMessage: ReadReceiptMessage; - let deliveryMessage: ReadReceiptMessage; let timestamps: Array; beforeEach(() => { timestamps = [987654321, 123456789]; const timestamp = Date.now(); readMessage = new ReadReceiptMessage({ timestamp, timestamps }); - deliveryMessage = new DeliveryReceiptMessage({ timestamp, timestamps }); }); it('content of a read receipt is correct', () => { @@ -28,18 +25,8 @@ describe('ReceiptMessage', () => { expect(decodedTimestamps).to.deep.equal(timestamps); }); - it('content of a delivery receipt is correct', () => { - const plainText = deliveryMessage.plainTextBuffer(); - const decoded = SignalService.Content.decode(plainText); - - expect(decoded.receiptMessage).to.have.property('type', 0); - const decodedTimestamps = (decoded.receiptMessage?.timestamp ?? []).map(toNumber); - expect(decodedTimestamps).to.deep.equal(timestamps); - }); - it('correct ttl', () => { expect(readMessage.ttl()).to.equal(Constants.TTL_DEFAULT.TTL_MAX); - expect(deliveryMessage.ttl()).to.equal(Constants.TTL_DEFAULT.TTL_MAX); }); it('has an identifier', () => {