Fix handling of replies on multiple dist lists
This commit is contained in:
parent
3c7502213b
commit
1941a33556
4 changed files with 284 additions and 70 deletions
|
@ -156,7 +156,7 @@ import { viewOnceOpenJobQueue } from '../jobs/viewOnceOpenJobQueue';
|
|||
import { getMessageIdForLogging } from '../util/idForLogging';
|
||||
import { hasAttachmentDownloads } from '../util/hasAttachmentDownloads';
|
||||
import { queueAttachmentDownloads } from '../util/queueAttachmentDownloads';
|
||||
import { findStoryMessage } from '../util/findStoryMessage';
|
||||
import { findStoryMessages } from '../util/findStoryMessage';
|
||||
import { getStoryDataFromMessageAttributes } from '../services/storyLoader';
|
||||
import type { ConversationQueueJobData } from '../jobs/conversationJobQueue';
|
||||
import { getMessageById } from '../messages/getMessageById';
|
||||
|
@ -2453,58 +2453,65 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
|
|||
);
|
||||
}
|
||||
|
||||
const [quote, storyQuote] = await Promise.all([
|
||||
const { storyContext } = initialMessage;
|
||||
let storyContextLogId = 'no storyContext';
|
||||
if (storyContext) {
|
||||
storyContextLogId =
|
||||
`storyContext(${storyContext.sentTimestamp}, ` +
|
||||
`${storyContext.authorUuid})`;
|
||||
}
|
||||
|
||||
const [quote, storyQuotes] = await Promise.all([
|
||||
this.copyFromQuotedMessage(initialMessage.quote, conversation.id),
|
||||
findStoryMessage(conversation.id, initialMessage.storyContext),
|
||||
findStoryMessages(conversation.id, storyContext),
|
||||
]);
|
||||
|
||||
if (initialMessage.storyContext && !storyQuote) {
|
||||
const storyQuote = storyQuotes.find(candidateQuote => {
|
||||
const sendStateByConversationId =
|
||||
candidateQuote.get('sendStateByConversationId') || {};
|
||||
const sendState = sendStateByConversationId[sender.id];
|
||||
|
||||
const storyQuoteIsFromSelf =
|
||||
candidateQuote.get('sourceUuid') ===
|
||||
window.storage.user.getCheckedUuid().toString();
|
||||
|
||||
if (!storyQuoteIsFromSelf) {
|
||||
return true;
|
||||
}
|
||||
if (sendState === undefined) {
|
||||
return false;
|
||||
}
|
||||
if (!isDirectConversation(conversation.attributes)) {
|
||||
return false;
|
||||
}
|
||||
return sendState.isAllowedToReplyToStory !== false;
|
||||
});
|
||||
|
||||
if (storyContext && !storyQuote) {
|
||||
if (!isDirectConversation(conversation.attributes)) {
|
||||
log.warn(
|
||||
`${idLog}: Received storyContext message in group but no matching story. Dropping.`
|
||||
`${idLog}: Received ${storyContextLogId} message in group but no matching story. Dropping.`
|
||||
);
|
||||
|
||||
confirm();
|
||||
return;
|
||||
}
|
||||
log.warn(
|
||||
`${idLog}: Received 1:1 storyContext message but no matching story. We'll try processing this message again later.`
|
||||
);
|
||||
|
||||
if (storyQuotes.length === 0) {
|
||||
log.warn(
|
||||
`${idLog}: Received ${storyContextLogId} message but no matching story. We'll try processing this message again later.`
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
log.warn(
|
||||
`${idLog}: Received ${storyContextLogId} message in 1:1 conversation but no matching story. Dropping.`
|
||||
);
|
||||
confirm();
|
||||
return;
|
||||
}
|
||||
|
||||
if (storyQuote) {
|
||||
const sendStateByConversationId =
|
||||
storyQuote.get('sendStateByConversationId') || {};
|
||||
const sendState = sendStateByConversationId[sender.id];
|
||||
|
||||
const storyQuoteIsFromSelf =
|
||||
storyQuote.get('sourceUuid') ===
|
||||
window.storage.user.getCheckedUuid().toString();
|
||||
|
||||
if (storyQuoteIsFromSelf && !sendState) {
|
||||
log.warn(
|
||||
`${idLog}: Received storyContext message but sender was not in sendStateByConversationId. Dropping.`
|
||||
);
|
||||
|
||||
confirm();
|
||||
return;
|
||||
}
|
||||
|
||||
if (
|
||||
storyQuoteIsFromSelf &&
|
||||
sendState.isAllowedToReplyToStory === false &&
|
||||
isDirectConversation(conversation.attributes)
|
||||
) {
|
||||
log.warn(
|
||||
`${idLog}: Received 1:1 storyContext message but sender is not allowed to reply. Dropping.`
|
||||
);
|
||||
|
||||
confirm();
|
||||
return;
|
||||
}
|
||||
|
||||
const storyDistributionListId = storyQuote.get(
|
||||
'storyDistributionListId'
|
||||
);
|
||||
|
@ -2517,7 +2524,7 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
|
|||
|
||||
if (!storyDistribution) {
|
||||
log.warn(
|
||||
`${idLog}: Received storyContext message for story with no associated distribution list. Dropping.`
|
||||
`${idLog}: Received ${storyContextLogId} message for story with no associated distribution list. Dropping.`
|
||||
);
|
||||
|
||||
confirm();
|
||||
|
@ -2526,7 +2533,7 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
|
|||
|
||||
if (!storyDistribution.allowsReplies) {
|
||||
log.warn(
|
||||
`${idLog}: Received storyContext message but distribution list does not allow replies. Dropping.`
|
||||
`${idLog}: Received ${storyContextLogId} message but distribution list does not allow replies. Dropping.`
|
||||
);
|
||||
|
||||
confirm();
|
||||
|
|
195
ts/test-mock/messaging/stories_test.ts
Normal file
195
ts/test-mock/messaging/stories_test.ts
Normal file
|
@ -0,0 +1,195 @@
|
|||
// Copyright 2023 Signal Messenger, LLC
|
||||
// SPDX-License-Identifier: AGPL-3.0-only
|
||||
|
||||
import createDebug from 'debug';
|
||||
import Long from 'long';
|
||||
import { Proto, StorageState } from '@signalapp/mock-server';
|
||||
|
||||
import * as durations from '../../util/durations';
|
||||
import { uuidToBytes } from '../../util/uuidToBytes';
|
||||
import { MY_STORY_ID } from '../../types/Stories';
|
||||
import { UUID } from '../../types/UUID';
|
||||
import type { App } from '../playwright';
|
||||
import { Bootstrap } from '../bootstrap';
|
||||
|
||||
export const debug = createDebug('mock:test:edit');
|
||||
|
||||
const IdentifierType = Proto.ManifestRecord.Identifier.Type;
|
||||
|
||||
const DISTRIBUTION1 = UUID.generate().toString();
|
||||
const DISTRIBUTION2 = UUID.generate().toString();
|
||||
|
||||
describe('story/messaging', function unknownContacts() {
|
||||
this.timeout(durations.MINUTE);
|
||||
|
||||
let bootstrap: Bootstrap;
|
||||
let app: App;
|
||||
|
||||
beforeEach(async () => {
|
||||
bootstrap = new Bootstrap();
|
||||
await bootstrap.init();
|
||||
|
||||
const { phone, contacts } = bootstrap;
|
||||
const [first, second] = contacts;
|
||||
|
||||
let state = StorageState.getEmpty();
|
||||
|
||||
state = state.updateAccount({
|
||||
profileKey: phone.profileKey.serialize(),
|
||||
e164: phone.device.number,
|
||||
givenName: phone.profileName,
|
||||
hasSetMyStoriesPrivacy: true,
|
||||
});
|
||||
|
||||
// Create empty My Story
|
||||
state = state.addRecord({
|
||||
type: IdentifierType.STORY_DISTRIBUTION_LIST,
|
||||
record: {
|
||||
storyDistributionList: {
|
||||
allowsReplies: true,
|
||||
identifier: uuidToBytes(MY_STORY_ID),
|
||||
isBlockList: false,
|
||||
name: MY_STORY_ID,
|
||||
recipientUuids: [],
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
// Create two distribution lists corresponding to two contacts
|
||||
state = state.addRecord({
|
||||
type: IdentifierType.STORY_DISTRIBUTION_LIST,
|
||||
record: {
|
||||
storyDistributionList: {
|
||||
allowsReplies: true,
|
||||
identifier: uuidToBytes(DISTRIBUTION1),
|
||||
isBlockList: false,
|
||||
name: 'first',
|
||||
recipientUuids: [first.device.uuid],
|
||||
},
|
||||
},
|
||||
});
|
||||
state = state.addRecord({
|
||||
type: IdentifierType.STORY_DISTRIBUTION_LIST,
|
||||
record: {
|
||||
storyDistributionList: {
|
||||
allowsReplies: true,
|
||||
identifier: uuidToBytes(DISTRIBUTION2),
|
||||
isBlockList: false,
|
||||
name: 'second',
|
||||
recipientUuids: [second.device.uuid],
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
// Finally whitelist and pin contacts
|
||||
for (const contact of [first, second]) {
|
||||
state = state.addContact(contact, {
|
||||
whitelisted: true,
|
||||
serviceE164: contact.device.number,
|
||||
identityKey: contact.publicKey.serialize(),
|
||||
profileKey: contact.profileKey.serialize(),
|
||||
givenName: contact.profileName,
|
||||
});
|
||||
state = state.pin(contact);
|
||||
}
|
||||
|
||||
await phone.setStorageState(state);
|
||||
app = await bootstrap.link();
|
||||
});
|
||||
|
||||
afterEach(async function after() {
|
||||
if (!bootstrap) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (this.currentTest?.state !== 'passed') {
|
||||
await bootstrap.saveLogs(app);
|
||||
}
|
||||
|
||||
await app.close();
|
||||
await bootstrap.teardown();
|
||||
});
|
||||
|
||||
it('allows replies on multiple distribution lists', async () => {
|
||||
const { phone, desktop, contacts } = bootstrap;
|
||||
const [first, second] = contacts;
|
||||
|
||||
const window = await app.getWindow();
|
||||
const sentAt = Date.now();
|
||||
|
||||
debug('waiting for storage service sync to complete');
|
||||
await app.waitForStorageService();
|
||||
|
||||
debug('sending story sync message');
|
||||
await phone.sendRaw(
|
||||
desktop,
|
||||
{
|
||||
syncMessage: {
|
||||
sent: {
|
||||
timestamp: Long.fromNumber(sentAt),
|
||||
expirationStartTimestamp: Long.fromNumber(sentAt),
|
||||
storyMessage: {
|
||||
textAttachment: {
|
||||
text: 'hello',
|
||||
},
|
||||
allowsReplies: true,
|
||||
},
|
||||
storyMessageRecipients: [
|
||||
{
|
||||
destinationUuid: first.device.uuid,
|
||||
distributionListIds: [DISTRIBUTION1],
|
||||
isAllowedToReply: true,
|
||||
},
|
||||
{
|
||||
destinationUuid: second.device.uuid,
|
||||
distributionListIds: [DISTRIBUTION2],
|
||||
isAllowedToReply: true,
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
},
|
||||
{ timestamp: sentAt }
|
||||
);
|
||||
|
||||
debug('sending story replies');
|
||||
await first.sendRaw(
|
||||
desktop,
|
||||
{
|
||||
dataMessage: {
|
||||
body: 'first reply',
|
||||
storyContext: {
|
||||
authorUuid: phone.device.uuid,
|
||||
sentTimestamp: Long.fromNumber(sentAt),
|
||||
},
|
||||
timestamp: Long.fromNumber(sentAt + 1),
|
||||
},
|
||||
},
|
||||
{ timestamp: sentAt + 1 }
|
||||
);
|
||||
await second.sendRaw(
|
||||
desktop,
|
||||
{
|
||||
dataMessage: {
|
||||
body: 'second reply',
|
||||
storyContext: {
|
||||
authorUuid: phone.device.uuid,
|
||||
sentTimestamp: Long.fromNumber(sentAt),
|
||||
},
|
||||
timestamp: Long.fromNumber(sentAt + 2),
|
||||
},
|
||||
},
|
||||
{ timestamp: sentAt + 2 }
|
||||
);
|
||||
|
||||
const leftPane = window.locator('.left-pane-wrapper');
|
||||
|
||||
debug('Finding both replies');
|
||||
await leftPane
|
||||
.locator(`[data-testid="${first.device.uuid}"] >> "first reply"`)
|
||||
.waitFor();
|
||||
await leftPane
|
||||
.locator(`[data-testid="${second.device.uuid}"] >> "second reply"`)
|
||||
.waitFor();
|
||||
});
|
||||
});
|
|
@ -38,7 +38,7 @@ import {
|
|||
SignedPreKeys,
|
||||
} from '../LibSignalStores';
|
||||
import { verifySignature } from '../Curve';
|
||||
import { strictAssert } from '../util/assert';
|
||||
import { strictAssert, assertDev } from '../util/assert';
|
||||
import type { BatcherType } from '../util/batcher';
|
||||
import { createBatcher } from '../util/batcher';
|
||||
import { drop } from '../util/drop';
|
||||
|
@ -2213,12 +2213,20 @@ export default class MessageReceiver
|
|||
'handleStoryMessage.destinationUuid'
|
||||
);
|
||||
|
||||
recipient.distributionListIds?.forEach(listId => {
|
||||
const sentUuids: Set<string> =
|
||||
distributionListToSentUuid.get(listId) || new Set();
|
||||
sentUuids.add(normalizedDestinationUuid);
|
||||
distributionListToSentUuid.set(listId, sentUuids);
|
||||
});
|
||||
if (recipient.distributionListIds) {
|
||||
recipient.distributionListIds.forEach(listId => {
|
||||
const sentUuids: Set<string> =
|
||||
distributionListToSentUuid.get(listId) || new Set();
|
||||
sentUuids.add(normalizedDestinationUuid);
|
||||
distributionListToSentUuid.set(listId, sentUuids);
|
||||
});
|
||||
} else {
|
||||
assertDev(
|
||||
false,
|
||||
`MessageReceiver.handleStoryMessage(${logId}): missing ` +
|
||||
`distribution list id for: ${destinationUuid}`
|
||||
);
|
||||
}
|
||||
|
||||
isAllowedToReply.set(
|
||||
normalizedDestinationUuid,
|
||||
|
|
|
@ -5,22 +5,22 @@ import type { MessageAttributesType } from '../model-types.d';
|
|||
import type { MessageModel } from '../models/messages';
|
||||
import type { SignalService as Proto } from '../protobuf';
|
||||
import * as log from '../logging/log';
|
||||
import { find } from './iterables';
|
||||
import { filter } from './iterables';
|
||||
import { getContactId } from '../messages/helpers';
|
||||
import { getTimestampFromLong } from './timestampLongUtils';
|
||||
|
||||
export async function findStoryMessage(
|
||||
export async function findStoryMessages(
|
||||
conversationId: string,
|
||||
storyContext?: Proto.DataMessage.IStoryContext
|
||||
): Promise<MessageModel | undefined> {
|
||||
): Promise<Array<MessageModel>> {
|
||||
if (!storyContext) {
|
||||
return;
|
||||
return [];
|
||||
}
|
||||
|
||||
const { authorUuid, sentTimestamp } = storyContext;
|
||||
|
||||
if (!authorUuid || !sentTimestamp) {
|
||||
return;
|
||||
return [];
|
||||
}
|
||||
|
||||
const sentAt = getTimestampFromLong(sentTimestamp);
|
||||
|
@ -28,33 +28,37 @@ export async function findStoryMessage(
|
|||
window.ConversationController.getOurConversationIdOrThrow();
|
||||
|
||||
const inMemoryMessages = window.MessageController.filterBySentAt(sentAt);
|
||||
const matchingMessage = find(inMemoryMessages, item =>
|
||||
isStoryAMatch(
|
||||
item.attributes,
|
||||
conversationId,
|
||||
ourConversationId,
|
||||
authorUuid,
|
||||
sentAt
|
||||
)
|
||||
);
|
||||
const matchingMessages = [
|
||||
...filter(inMemoryMessages, item =>
|
||||
isStoryAMatch(
|
||||
item.attributes,
|
||||
conversationId,
|
||||
ourConversationId,
|
||||
authorUuid,
|
||||
sentAt
|
||||
)
|
||||
),
|
||||
];
|
||||
|
||||
if (matchingMessage) {
|
||||
return matchingMessage;
|
||||
if (matchingMessages.length > 0) {
|
||||
return matchingMessages;
|
||||
}
|
||||
|
||||
log.info('findStoryMessage: db lookup needed', sentAt);
|
||||
log.info('findStoryMessages: db lookup needed', sentAt);
|
||||
const messages = await window.Signal.Data.getMessagesBySentAt(sentAt);
|
||||
const found = messages.find(item =>
|
||||
const found = messages.filter(item =>
|
||||
isStoryAMatch(item, conversationId, ourConversationId, authorUuid, sentAt)
|
||||
);
|
||||
|
||||
if (!found) {
|
||||
log.info('findStoryMessage: message not found', sentAt);
|
||||
return;
|
||||
if (found.length !== 0) {
|
||||
log.info('findStoryMessages: message not found', sentAt);
|
||||
return [];
|
||||
}
|
||||
|
||||
const message = window.MessageController.register(found.id, found);
|
||||
return message;
|
||||
const result = found.map(attributes =>
|
||||
window.MessageController.register(attributes.id, attributes)
|
||||
);
|
||||
return result;
|
||||
}
|
||||
|
||||
function isStoryAMatch(
|
||||
|
|
Loading…
Reference in a new issue