diff --git a/ts/models/messages.ts b/ts/models/messages.ts index f5c7f2a6c5..78f2fc1ff5 100644 --- a/ts/models/messages.ts +++ b/ts/models/messages.ts @@ -2530,12 +2530,19 @@ export class MessageModel extends window.Backbone.Model { const urls = LinkPreview.findLinks(dataMessage.body || ''); const incomingPreview = dataMessage.preview || []; - const preview = incomingPreview.filter( - (item: LinkPreviewType) => - (item.image || item.title) && - urls.includes(item.url) && - LinkPreview.shouldPreviewHref(item.url) - ); + const preview = incomingPreview.filter((item: LinkPreviewType) => { + if (!item.image && !item.title) { + return false; + } + // Story link previews don't have to correspond to links in the + // message body. + if (isStory(message.attributes)) { + return true; + } + return ( + urls.includes(item.url) && LinkPreview.shouldPreviewHref(item.url) + ); + }); if (preview.length < incomingPreview.length) { log.info( `${message.idForLogging()}: Eliminated ${ diff --git a/ts/services/storyLoader.ts b/ts/services/storyLoader.ts index 1befcf2b4e..308de2ed8a 100644 --- a/ts/services/storyLoader.ts +++ b/ts/services/storyLoader.ts @@ -7,7 +7,11 @@ import type { StoryDataType } from '../state/ducks/stories'; import * as durations from '../util/durations'; import * as log from '../logging/log'; import dataInterface from '../sql/Client'; -import { getAttachmentsForMessage } from '../state/selectors/message'; +import { + getAttachmentsForMessage, + getPropsForAttachment, +} from '../state/selectors/message'; +import type { LinkPreviewType } from '../types/message/LinkPreviews'; import { isNotNil } from '../util/isNotNil'; import { strictAssert } from '../util/assert'; import { dropNull } from '../util/dropNull'; @@ -66,11 +70,38 @@ export function getStoryDataFromMessageAttributes( return; } - const [attachment] = + let [attachment] = unresolvedAttachment && unresolvedAttachment.path ? getAttachmentsForMessage(message) : [unresolvedAttachment]; + let preview: LinkPreviewType | undefined; + if (message.preview?.length) { + strictAssert( + message.preview.length === 1, + 'getStoryDataFromMessageAttributes: story can have only one preview' + ); + [preview] = message.preview; + + strictAssert( + attachment?.textAttachment, + 'getStoryDataFromMessageAttributes: story must have a ' + + 'textAttachment with preview' + ); + attachment = { + ...attachment, + textAttachment: { + ...attachment.textAttachment, + preview: { + ...preview, + image: preview.image && getPropsForAttachment(preview.image), + }, + }, + }; + } else if (attachment) { + attachment = getPropsForAttachment(attachment); + } + return { attachment, messageId: message.id, diff --git a/ts/state/ducks/stories.ts b/ts/state/ducks/stories.ts index 6bc86323c4..73aa34f7ef 100644 --- a/ts/state/ducks/stories.ts +++ b/ts/state/ducks/stories.ts @@ -32,17 +32,15 @@ import { markViewed } from '../../services/MessageUpdater'; import { queueAttachmentDownloads } from '../../util/queueAttachmentDownloads'; import { replaceIndex } from '../../util/replaceIndex'; import { showToast } from '../../util/showToast'; -import { - hasFailed, - hasNotResolved, - isDownloaded, - isDownloading, -} from '../../types/Attachment'; +import { hasFailed, isDownloaded, isDownloading } from '../../types/Attachment'; import { getConversationSelector, getHideStoryConversationIds, } from '../selectors/conversations'; -import { getStories } from '../selectors/stories'; +import { + getStories, + getStoryDownloadableAttachment, +} from '../selectors/stories'; import { getStoryDataFromMessageAttributes } from '../../services/storyLoader'; import { isGroup } from '../../util/whatTypeOfConversation'; import { isNotNil } from '../../util/isNotNil'; @@ -113,7 +111,6 @@ const LIST_MEMBERS_VERIFIED = 'stories/LIST_MEMBERS_VERIFIED'; const LOAD_STORY_REPLIES = 'stories/LOAD_STORY_REPLIES'; const MARK_STORY_READ = 'stories/MARK_STORY_READ'; const QUEUE_STORY_DOWNLOAD = 'stories/QUEUE_STORY_DOWNLOAD'; -export const RESOLVE_ATTACHMENT_URL = 'stories/RESOLVE_ATTACHMENT_URL'; const SEND_STORY_MODAL_OPEN_STATE_CHANGED = 'stories/SEND_STORY_MODAL_OPEN_STATE_CHANGED'; const STORY_CHANGED = 'stories/STORY_CHANGED'; @@ -155,14 +152,6 @@ type QueueStoryDownloadActionType = { payload: string; }; -type ResolveAttachmentUrlActionType = { - type: typeof RESOLVE_ATTACHMENT_URL; - payload: { - messageId: string; - attachmentUrl: string; - }; -}; - type SendStoryModalOpenStateChanged = { type: typeof SEND_STORY_MODAL_OPEN_STATE_CHANGED; payload: number | undefined; @@ -195,7 +184,6 @@ export type StoriesActionType = | MessageDeletedActionType | MessagesAddedActionType | QueueStoryDownloadActionType - | ResolveAttachmentUrlActionType | SendStoryModalOpenStateChanged | StoryChangedActionType | ToggleViewActionType @@ -337,7 +325,7 @@ function queueStoryDownload( void, RootStateType, unknown, - NoopActionType | QueueStoryDownloadActionType | ResolveAttachmentUrlActionType + NoopActionType | QueueStoryDownloadActionType > { return async (dispatch, getState) => { const { stories } = getState().stories; @@ -347,7 +335,7 @@ function queueStoryDownload( return; } - const { attachment } = story; + const attachment = getStoryDownloadableAttachment(story); if (!attachment) { log.warn('queueStoryDownload: No attachment found for story', { @@ -365,21 +353,6 @@ function queueStoryDownload( return; } - // This function also resolves the attachment's URL in case we've already - // downloaded the attachment but haven't pointed its path to an absolute - // location on disk. - if (hasNotResolved(attachment)) { - dispatch({ - type: RESOLVE_ATTACHMENT_URL, - payload: { - messageId: storyId, - attachmentUrl: window.Signal.Migrations.getAbsoluteAttachmentPath( - attachment.path - ), - }, - }); - } - return; } @@ -403,7 +376,10 @@ function queueStoryDownload( payload: storyId, }); - await queueAttachmentDownloads(message.attributes); + const updatedFields = await queueAttachmentDownloads(message.attributes); + if (updatedFields) { + message.set(updatedFields); + } return; } @@ -627,11 +603,7 @@ const getSelectedStoryDataForDistributionListId = ( }; const getSelectedStoryDataForConversationId = ( - dispatch: ThunkDispatch< - RootStateType, - unknown, - NoopActionType | ResolveAttachmentUrlActionType - >, + dispatch: ThunkDispatch, getState: () => RootStateType, conversationId: string, selectedStoryId?: string @@ -671,12 +643,12 @@ const getSelectedStoryDataForConversationId = ( const numStories = storiesByConversationId.length; // Queue all undownloaded stories once we're viewing someone's stories - storiesByConversationId.forEach(item => { - if (isDownloaded(item.attachment) || isDownloading(item.attachment)) { + storiesByConversationId.forEach(({ attachment, messageId }) => { + if (isDownloaded(attachment) || isDownloading(attachment)) { return; } - queueStoryDownload(item.messageId)(dispatch, getState, null); + queueStoryDownload(messageId)(dispatch, getState, null); }); return { @@ -1416,41 +1388,6 @@ export function reducer( }; } - if (action.type === RESOLVE_ATTACHMENT_URL) { - const { messageId, attachmentUrl } = action.payload; - - const storyIndex = state.stories.findIndex( - existingStory => existingStory.messageId === messageId - ); - - if (storyIndex < 0) { - return state; - } - - const story = state.stories[storyIndex]; - - if (!story.attachment) { - return state; - } - - const storyWithResolvedAttachment = { - ...story, - attachment: { - ...story.attachment, - url: attachmentUrl, - }, - }; - - return { - ...state, - stories: replaceIndex( - state.stories, - storyIndex, - storyWithResolvedAttachment - ), - }; - } - if (action.type === DOE_STORY) { return { ...state, diff --git a/ts/state/selectors/stories.ts b/ts/state/selectors/stories.ts index 12d7b6c40e..5375923c75 100644 --- a/ts/state/selectors/stories.ts +++ b/ts/state/selectors/stories.ts @@ -7,6 +7,7 @@ import { pick } from 'lodash'; import type { GetConversationByIdType } from './conversations'; import type { ConversationType } from '../ducks/conversations'; import type { MessageReactionType } from '../../model-types.d'; +import type { AttachmentType } from '../../types/Attachment'; import type { ConversationStoryType, MyStoryType, @@ -133,6 +134,13 @@ function getAvatarData( ]); } +export function getStoryDownloadableAttachment({ + attachment, +}: StoryDataType): AttachmentType | undefined { + // See: getStoryDataFromMessageAttributes for how preview gets populated. + return attachment?.textAttachment?.preview?.image ?? attachment; +} + export function getStoryView( conversationSelector: GetConversationByIdType, ourConversationId: string | undefined, @@ -159,13 +167,7 @@ export function getStoryView( expireTimer, readAt, timestamp, - } = pick(story, [ - 'attachment', - 'expirationStartTimestamp', - 'expireTimer', - 'readAt', - 'timestamp', - ]); + } = story; const { sendStateByConversationId } = story; let sendState: Array | undefined; diff --git a/ts/test-electron/state/ducks/stories_test.ts b/ts/test-electron/state/ducks/stories_test.ts index 6fbbc2f179..98b6eaa70a 100644 --- a/ts/test-electron/state/ducks/stories_test.ts +++ b/ts/test-electron/state/ducks/stories_test.ts @@ -3,12 +3,9 @@ import * as sinon from 'sinon'; import casual from 'casual'; -import path from 'path'; -import { assert } from 'chai'; import type { DispatchableViewStoryType, - StoriesStateType, StoryDataType, } from '../../../state/ducks/stories'; import type { ConversationType } from '../../../state/ducks/conversations'; @@ -16,19 +13,14 @@ import type { MessageAttributesType } from '../../../model-types.d'; import type { StateType as RootStateType } from '../../../state/reducer'; import type { UUIDStringType } from '../../../types/UUID'; import { DAY } from '../../../util/durations'; -import { IMAGE_JPEG } from '../../../types/MIME'; +import { TEXT_ATTACHMENT, IMAGE_JPEG } from '../../../types/MIME'; import { ReadStatus } from '../../../messages/MessageReadStatus'; import { StoryViewDirectionType, StoryViewModeType, } from '../../../types/Stories'; import { UUID } from '../../../types/UUID'; -import { - actions, - getEmptyState, - reducer, - RESOLVE_ATTACHMENT_URL, -} from '../../../state/ducks/stories'; +import { actions, getEmptyState } from '../../../state/ducks/stories'; import { noopAction } from '../../../state/ducks/noop'; import { reducer as rootReducer } from '../../../state/reducer'; import { dropNull } from '../../../util/dropNull'; @@ -917,86 +909,6 @@ describe('both/state/ducks/stories', () => { sinon.assert.notCalled(dispatch); }); - it('downloaded, but unresolved, we should resolve the path', async function test() { - const storyId = UUID.generate().toString(); - const attachment = { - contentType: IMAGE_JPEG, - path: 'image.jpg', - size: 0, - }; - const messageAttributes = { - ...getStoryMessage(storyId), - attachments: [attachment], - }; - - const rootState = getEmptyRootState(); - - const getState = () => ({ - ...rootState, - stories: { - ...rootState.stories, - stories: [ - { - ...messageAttributes, - attachment: messageAttributes.attachments[0], - messageId: messageAttributes.id, - expireTimer: messageAttributes.expireTimer, - expirationStartTimestamp: dropNull( - messageAttributes.expirationStartTimestamp - ), - }, - ], - }, - }); - - window.MessageController.register(storyId, messageAttributes); - - const dispatch = sinon.spy(); - await queueStoryDownload(storyId)(dispatch, getState, null); - - const action = dispatch.getCall(0).args[0]; - - sinon.assert.calledWith(dispatch, { - type: RESOLVE_ATTACHMENT_URL, - payload: { - messageId: storyId, - attachmentUrl: action.payload.attachmentUrl, - }, - }); - assert.equal( - attachment.path, - path.basename(action.payload.attachmentUrl) - ); - - const stateWithStory: StoriesStateType = { - ...getEmptyRootState().stories, - stories: [ - { - ...messageAttributes, - messageId: storyId, - attachment, - expireTimer: messageAttributes.expireTimer, - expirationStartTimestamp: dropNull( - messageAttributes.expirationStartTimestamp - ), - }, - ], - }; - - const nextState = reducer(stateWithStory, action); - assert.isDefined(nextState.stories); - assert.equal( - nextState.stories[0].attachment?.url, - action.payload.attachmentUrl - ); - - const state = getEmptyRootState().stories; - - const sameState = reducer(state, action); - assert.isDefined(sameState.stories); - assert.equal(sameState, state); - }); - it('not downloaded, queued for download', async function test() { const storyId = UUID.generate().toString(); const messageAttributes = { @@ -1039,5 +951,59 @@ describe('both/state/ducks/stories', () => { payload: storyId, }); }); + + it('preview not downloaded, queued for download', async function test() { + const storyId = UUID.generate().toString(); + const preview = { + url: 'https://signal.org', + image: { + contentType: IMAGE_JPEG, + size: 0, + }, + }; + const messageAttributes = { + ...getStoryMessage(storyId), + attachments: [ + { + contentType: TEXT_ATTACHMENT, + size: 0, + textAttachment: { + preview, + }, + }, + ], + preview: [preview], + }; + + const rootState = getEmptyRootState(); + + const getState = () => ({ + ...rootState, + stories: { + ...rootState.stories, + stories: [ + { + ...messageAttributes, + attachment: messageAttributes.attachments[0], + messageId: messageAttributes.id, + expireTimer: messageAttributes.expireTimer, + expirationStartTimestamp: dropNull( + messageAttributes.expirationStartTimestamp + ), + }, + ], + }, + }); + + window.MessageController.register(storyId, messageAttributes); + + const dispatch = sinon.spy(); + await queueStoryDownload(storyId)(dispatch, getState, null); + + sinon.assert.calledWith(dispatch, { + type: 'stories/QUEUE_STORY_DOWNLOAD', + payload: storyId, + }); + }); }); }); diff --git a/ts/textsecure/MessageReceiver.ts b/ts/textsecure/MessageReceiver.ts index 198e21c476..4cf5e0371a 100644 --- a/ts/textsecure/MessageReceiver.ts +++ b/ts/textsecure/MessageReceiver.ts @@ -3,7 +3,7 @@ /* eslint-disable no-bitwise */ -import { isBoolean, isNumber } from 'lodash'; +import { isBoolean, isNumber, omit } from 'lodash'; import PQueue from 'p-queue'; import { v4 as getGuid } from 'uuid'; @@ -60,6 +60,7 @@ import createTaskWithTimeout from './TaskWithTimeout'; import { processAttachment, processDataMessage, + processPreview, processGroupV2Context, } from './processDataMessage'; import { processSyncMessage } from './processSyncMessage'; @@ -75,6 +76,7 @@ import * as Bytes from '../Bytes'; import type { ProcessedAttachment, ProcessedDataMessage, + ProcessedPreview, ProcessedSyncMessage, ProcessedSent, ProcessedEnvelope, @@ -1993,6 +1995,7 @@ export default class MessageReceiver log.info('MessageReceiver.handleStoryMessage', logId); const attachments: Array = []; + let preview: ReadonlyArray | undefined; if (msg.fileAttachment) { const attachment = processAttachment(msg.fileAttachment); @@ -2000,16 +2003,17 @@ export default class MessageReceiver } if (msg.textAttachment) { - const { text } = msg.textAttachment; - if (!text) { - throw new Error('Text attachments must have text!'); + const { text, preview: unprocessedPreview } = msg.textAttachment; + if (unprocessedPreview) { + preview = processPreview([unprocessedPreview]); + } else if (!text) { + throw new Error('Text attachments must have text or link preview!'); } - // TODO DESKTOP-3714 we should download the story link preview image attachments.push({ - size: text.length, + size: text?.length ?? 0, contentType: TEXT_ATTACHMENT, - textAttachment: msg.textAttachment, + textAttachment: omit(msg.textAttachment, 'preview'), blurHash: generateBlurHash( (msg.textAttachment.color || msg.textAttachment.gradient?.startColor) ?? @@ -2045,6 +2049,7 @@ export default class MessageReceiver const message: ProcessedDataMessage = { attachments, + preview, canReplyToStory: Boolean(msg.allowsReplies), expireTimer: durations.DAY / 1000, flags: 0, diff --git a/ts/types/Attachment.ts b/ts/types/Attachment.ts index 02e8fe4c84..c75bdb010d 100644 --- a/ts/types/Attachment.ts +++ b/ts/types/Attachment.ts @@ -697,20 +697,33 @@ export function isGIF(attachments?: ReadonlyArray): boolean { return hasFlag && isVideoAttachment(attachment); } +function resolveNestedAttachment( + attachment?: AttachmentType +): AttachmentType | undefined { + if (attachment?.textAttachment?.preview?.image) { + return attachment.textAttachment.preview.image; + } + return attachment; +} + export function isDownloaded(attachment?: AttachmentType): boolean { - return Boolean(attachment && (attachment.path || attachment.textAttachment)); + const resolved = resolveNestedAttachment(attachment); + return Boolean(resolved && (resolved.path || resolved.textAttachment)); } export function hasNotResolved(attachment?: AttachmentType): boolean { - return Boolean(attachment && !attachment.url && !attachment.textAttachment); + const resolved = resolveNestedAttachment(attachment); + return Boolean(resolved && !resolved.url && !resolved.textAttachment); } export function isDownloading(attachment?: AttachmentType): boolean { - return Boolean(attachment && attachment.downloadJobId && attachment.pending); + const resolved = resolveNestedAttachment(attachment); + return Boolean(resolved && resolved.downloadJobId && resolved.pending); } export function hasFailed(attachment?: AttachmentType): boolean { - return Boolean(attachment && attachment.error); + const resolved = resolveNestedAttachment(attachment); + return Boolean(resolved && resolved.error); } export function hasVideoBlurHash(attachments?: Array): boolean {