Improve quoted attachment typings

This commit is contained in:
trevor-signal 2024-05-23 17:06:41 -04:00 committed by GitHub
parent 38226115a4
commit 5f0080a7d7
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
20 changed files with 98 additions and 97 deletions

View file

@ -37,7 +37,7 @@ import { ImageGrid } from './ImageGrid';
import { GIF } from './GIF';
import { CurveType, Image } from './Image';
import { ContactName } from './ContactName';
import type { QuotedAttachmentType } from './Quote';
import type { QuotedAttachmentForUIType } from './Quote';
import { Quote } from './Quote';
import { EmbeddedContact } from './EmbeddedContact';
import type { OwnProps as ReactionViewerProps } from './ReactionViewer';
@ -247,7 +247,7 @@ export type PropsData = {
conversationTitle: string;
customColor?: CustomColorType;
text: string;
rawAttachment?: QuotedAttachmentType;
rawAttachment?: QuotedAttachmentForUIType;
payment?: AnyPaymentEvent;
isFromMe: boolean;
sentAt: number;
@ -267,7 +267,7 @@ export type PropsData = {
customColor?: CustomColorType;
emoji?: string;
isFromMe: boolean;
rawAttachment?: QuotedAttachmentType;
rawAttachment?: QuotedAttachmentForUIType;
storyId?: string;
text: string;
};

View file

@ -235,6 +235,7 @@ ImageOnly.args = {
contentType: IMAGE_PNG,
height: 100,
width: 100,
size: 100,
path: pngUrl,
objectUrl: pngUrl,
},
@ -251,6 +252,7 @@ ImageAttachment.args = {
contentType: IMAGE_PNG,
height: 100,
width: 100,
size: 100,
path: pngUrl,
objectUrl: pngUrl,
},
@ -287,6 +289,7 @@ VideoOnly.args = {
contentType: IMAGE_PNG,
height: 100,
width: 100,
size: 100,
path: pngUrl,
objectUrl: pngUrl,
},
@ -304,6 +307,7 @@ VideoAttachment.args = {
contentType: IMAGE_PNG,
height: 100,
width: 100,
size: 100,
path: pngUrl,
objectUrl: pngUrl,
},
@ -509,6 +513,7 @@ IsStoryReplyEmoji.args = {
contentType: IMAGE_PNG,
height: 100,
width: 100,
size: 100,
path: pngUrl,
objectUrl: pngUrl,
},

View file

@ -26,9 +26,13 @@ import type { AnyPaymentEvent } from '../../types/Payment';
import { PaymentEventKind } from '../../types/Payment';
import { getPaymentEventNotificationText } from '../../messages/helpers';
import { RenderLocation } from './MessageTextRenderer';
import type { QuotedAttachmentType } from '../../model-types';
const EMPTY_OBJECT = Object.freeze(Object.create(null));
export type QuotedAttachmentForUIType = QuotedAttachmentType &
Pick<AttachmentType, 'isVoiceMessage' | 'fileName' | 'textAttachment'>;
export type Props = {
authorTitle: string;
conversationColor: ConversationColorType;
@ -44,7 +48,7 @@ export type Props = {
onClick?: () => void;
onClose?: () => void;
text: string;
rawAttachment?: QuotedAttachmentType;
rawAttachment?: QuotedAttachmentForUIType;
payment?: AnyPaymentEvent;
isGiftBadge: boolean;
isViewOnce: boolean;
@ -53,11 +57,6 @@ export type Props = {
doubleCheckMissingQuoteReference?: () => unknown;
};
export type QuotedAttachmentType = Pick<
AttachmentType,
'contentType' | 'fileName' | 'isVoiceMessage' | 'thumbnail' | 'textAttachment'
>;
function validateQuote(quote: Props): boolean {
if (
quote.isStoryReply &&
@ -86,9 +85,9 @@ function validateQuote(quote: Props): boolean {
}
// Long message attachments should not be shown.
function getAttachment(
rawAttachment: undefined | QuotedAttachmentType
): undefined | QuotedAttachmentType {
function getAttachment<T extends Pick<QuotedAttachmentType, 'contentType'>>(
rawAttachment: T | undefined
): T | undefined {
return rawAttachment && !MIME.isLongMessage(rawAttachment.contentType)
? rawAttachment
: undefined;

View file

@ -779,8 +779,7 @@ async function uploadMessageQuote({
prop: 'quote',
targetTimestamp,
});
const loadedQuote =
message.cachedOutgoingQuoteData || (await loadQuoteData(startingQuote));
const loadedQuote = await loadQuoteData(startingQuote);
if (!loadedQuote) {
return undefined;
@ -797,7 +796,10 @@ async function uploadMessageQuote({
};
}
const uploaded = await uploadAttachment(thumbnail);
const { data } = thumbnail;
strictAssert(data, 'data must be loaded into thumbnail');
const uploaded = await uploadAttachment({ ...thumbnail, data });
return {
contentType: attachment.contentType,
@ -826,15 +828,11 @@ async function uploadMessageQuote({
}
strictAssert(
attachment.path === loadedQuote.attachments.at(index)?.path,
attachment.thumbnail.path ===
loadedQuote.attachments.at(index)?.thumbnail?.path,
`${logId}: Quote attachment ${index} was updated from under us`
);
strictAssert(
attachment.thumbnail,
`${logId}: Quote attachment ${index} no longer has a thumbnail`
);
const attachmentAfterThumbnailUpload =
attachmentsAfterThumbnailUpload[index];
return {

View file

@ -274,7 +274,7 @@ export async function addAttachmentToMessage(
attachments: edit.quote.attachments.map(item => {
const { thumbnail } = item;
if (!thumbnail) {
return;
return item;
}
const newThumbnail = maybeReplaceAttachment(thumbnail);

13
ts/model-types.d.ts vendored
View file

@ -14,7 +14,11 @@ import type { ReadStatus } from './messages/MessageReadStatus';
import type { SendStateByConversationId } from './messages/MessageSendState';
import type { GroupNameCollisionsWithIdsByTitle } from './util/groupMemberNameCollisions';
import type { AttachmentDraftType, AttachmentType } from './types/Attachment';
import type {
AttachmentDraftType,
AttachmentType,
ThumbnailType,
} from './types/Attachment';
import type { EmbeddedContactType } from './types/EmbeddedContact';
import { SignalService as Proto } from './protobuf';
import type { AvatarDataType, ContactAvatarType } from './types/Avatar';
@ -73,16 +77,15 @@ export type GroupMigrationType = {
invitedMemberCount?: number;
};
export type QuotedAttachment = {
export type QuotedAttachmentType = {
contentType: MIMEType;
fileName?: string;
thumbnail?: AttachmentType;
thumbnail?: ThumbnailType;
};
export type QuotedMessageType = {
// TODO DESKTOP-3826
// eslint-disable-next-line @typescript-eslint/no-explicit-any
attachments: ReadonlyArray<any>;
attachments: ReadonlyArray<QuotedAttachmentType>;
payment?: AnyPaymentEvent;
// `author` is an old attribute that holds the author's E164. We shouldn't use it for
// new messages, but old messages might have this attribute.

View file

@ -3967,7 +3967,6 @@ export class ConversationModel extends window.Backbone
};
});
}
message.cachedOutgoingQuoteData = quote;
message.cachedOutgoingStickerData = sticker;
const dbStart = Date.now();

View file

@ -37,7 +37,6 @@ import type {
} from '../textsecure/Types.d';
import { SendMessageProtoError } from '../textsecure/Errors';
import { getUserLanguages } from '../util/userLanguages';
import { copyCdnFields } from '../util/attachments';
import type { ReactionType } from '../types/Reactions';
import { ReactionReadStatus } from '../types/Reactions';
@ -189,8 +188,6 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
cachedOutgoingPreviewData?: Array<LinkPreviewWithHydratedData>;
cachedOutgoingQuoteData?: QuotedMessageType;
cachedOutgoingStickerData?: StickerWithHydratedData;
public registerLocations: Set<string>;
@ -849,7 +846,6 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
// We aren't trying to send this message anymore, so we'll delete these caches
delete this.cachedOutgoingContactData;
delete this.cachedOutgoingPreviewData;
delete this.cachedOutgoingQuoteData;
delete this.cachedOutgoingStickerData;
this.notifyStorySendFailed();
@ -1127,7 +1123,6 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
if (isTotalSuccess) {
delete this.cachedOutgoingContactData;
delete this.cachedOutgoingPreviewData;
delete this.cachedOutgoingQuoteData;
delete this.cachedOutgoingStickerData;
}
@ -1486,7 +1481,6 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
): Promise<void> {
const { attachments } = quote;
const firstAttachment = attachments ? attachments[0] : undefined;
const firstThumbnailCdnFields = copyCdnFields(firstAttachment?.thumbnail);
if (messageHasPaymentEvent(originalMessage.attributes)) {
// eslint-disable-next-line no-param-reassign
@ -1535,7 +1529,7 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
quote.bodyRanges = originalMessage.attributes.bodyRanges;
if (firstAttachment) {
firstAttachment.thumbnail = null;
firstAttachment.thumbnail = undefined;
}
if (!firstAttachment || !firstAttachment.contentType) {
@ -1571,14 +1565,13 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
if (thumbnail && thumbnail.path) {
firstAttachment.thumbnail = {
...firstThumbnailCdnFields,
...thumbnail,
copied: true,
};
} else {
firstAttachment.contentType = queryFirst.contentType;
firstAttachment.fileName = queryFirst.fileName;
firstAttachment.thumbnail = null;
firstAttachment.thumbnail = undefined;
}
}
@ -1589,7 +1582,6 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
if (image && image.path) {
firstAttachment.thumbnail = {
...firstThumbnailCdnFields,
...image,
copied: true,
};
@ -1599,7 +1591,6 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
const sticker = originalMessage.get('sticker');
if (sticker && sticker.data && sticker.data.path) {
firstAttachment.thumbnail = {
...firstThumbnailCdnFields,
...sticker.data,
copied: true,
};

View file

@ -24,7 +24,7 @@ import { PaymentEventKind } from '../../types/Payment';
import type {
ConversationAttributesType,
MessageAttributesType,
QuotedAttachment,
QuotedAttachmentType,
QuotedMessageType,
} from '../../model-types.d';
import { drop } from '../../util/drop';
@ -1622,7 +1622,7 @@ export class BackupExportStream extends Readable {
: null,
authorId,
text: quote.text,
attachments: quote.attachments.map((attachment: QuotedAttachment) => {
attachments: quote.attachments.map((attachment: QuotedAttachmentType) => {
return {
contentType: attachment.contentType,
fileName: attachment.fileName,

View file

@ -13,6 +13,7 @@ import type {
LastMessageStatus,
MessageAttributesType,
MessageReactionType,
QuotedAttachmentType,
ShallowChallengeError,
} from '../../model-types.d';
@ -41,7 +42,6 @@ import type {
ChangeType,
} from '../../components/conversation/GroupNotification';
import type { PropsType as ProfileChangeNotificationPropsType } from '../../components/conversation/ProfileChangeNotification';
import type { QuotedAttachmentType } from '../../components/conversation/Quote';
import { getDomain, isCallLink, isStickerPack } from '../../types/LinkPreview';
import type {
@ -1852,9 +1852,7 @@ export function getPropsForAttachment(
};
}
function processQuoteAttachment(
attachment: AttachmentType
): QuotedAttachmentType {
function processQuoteAttachment(attachment: QuotedAttachmentType) {
const { thumbnail } = attachment;
const path =
thumbnail && thumbnail.path && getAttachmentUrlForPath(thumbnail.path);

View file

@ -26,6 +26,7 @@ export const fakeThumbnail = (url: string): ThumbnailType => ({
path: url,
url,
width: 100,
size: 128,
});
export const fakeDraftAttachment = (

View file

@ -10,7 +10,7 @@ import {
} from '../textsecure/processDataMessage';
import type { ProcessedAttachment } from '../textsecure/Types.d';
import { SignalService as Proto } from '../protobuf';
import { IMAGE_GIF } from '../types/MIME';
import { IMAGE_GIF, IMAGE_JPEG } from '../types/MIME';
import { generateAci } from '../types/ServiceId';
const ACI_1 = generateAci();
@ -142,7 +142,7 @@ describe('processDataMessage', () => {
text: 'text',
attachments: [
{
contentType: 'image/jpeg',
contentType: IMAGE_JPEG,
fileName: 'image.jpg',
thumbnail: PROCESSED_ATTACHMENT,
},

View file

@ -174,9 +174,12 @@ describe('Message', () => {
referencedMessageNotFound: false,
attachments: [
{
contentType: MIME.APPLICATION_OCTET_STREAM,
thumbnail: {
path: 'ab/abcdefghi',
data: Bytes.fromString('Its easy if you try'),
contentType: MIME.APPLICATION_OCTET_STREAM,
size: 128,
},
},
],
@ -193,8 +196,11 @@ describe('Message', () => {
referencedMessageNotFound: false,
attachments: [
{
contentType: MIME.APPLICATION_OCTET_STREAM,
thumbnail: {
path: 'ab/abcdefghi',
contentType: MIME.APPLICATION_OCTET_STREAM,
size: 128,
},
},
],
@ -707,7 +713,7 @@ describe('Message', () => {
attachments: [
{
fileName: 'manifesto.txt',
contentType: 'text/plain',
contentType: MIME.TEXT_ATTACHMENT,
},
],
id: 34233,
@ -726,7 +732,7 @@ describe('Message', () => {
it('does not eliminate thumbnails with missing data field', async () => {
const upgradeAttachment = sinon
.stub()
.returns({ fileName: 'processed!' });
.returns({ contentType: MIME.IMAGE_GIF, size: 42 });
const upgradeVersion = Message._mapQuotedAttachments(upgradeAttachment);
const message = getDefaultMessage({
@ -736,9 +742,10 @@ describe('Message', () => {
attachments: [
{
fileName: 'cat.gif',
contentType: 'image/gif',
contentType: MIME.IMAGE_GIF,
thumbnail: {
fileName: 'not yet downloaded!',
contentType: MIME.IMAGE_GIF,
size: 128,
},
},
],
@ -754,10 +761,11 @@ describe('Message', () => {
text: 'hey!',
attachments: [
{
contentType: 'image/gif',
contentType: MIME.IMAGE_GIF,
fileName: 'cat.gif',
thumbnail: {
fileName: 'processed!',
contentType: MIME.IMAGE_GIF,
size: 42,
},
},
],
@ -777,6 +785,8 @@ describe('Message', () => {
it('calls provided async function for each quoted attachment', async () => {
const upgradeAttachment = sinon.stub().resolves({
path: '/new/path/on/disk',
contentType: MIME.TEXT_ATTACHMENT,
size: 100,
});
const upgradeVersion = Message._mapQuotedAttachments(upgradeAttachment);
@ -786,8 +796,11 @@ describe('Message', () => {
text: 'hey!',
attachments: [
{
contentType: MIME.TEXT_ATTACHMENT,
thumbnail: {
data: 'data is here',
contentType: MIME.TEXT_ATTACHMENT,
size: 100,
data: Buffer.from('data is here'),
},
},
],
@ -803,7 +816,10 @@ describe('Message', () => {
text: 'hey!',
attachments: [
{
contentType: MIME.TEXT_ATTACHMENT,
thumbnail: {
contentType: MIME.TEXT_ATTACHMENT,
size: 100,
path: '/new/path/on/disk',
},
},

View file

@ -132,7 +132,7 @@ export type ProcessedGroupV2Context = {
};
export type ProcessedQuoteAttachment = {
contentType?: string;
contentType: MIMEType;
fileName?: string;
thumbnail?: ProcessedAttachment;
};

View file

@ -142,7 +142,9 @@ export function processQuote(
text: dropNull(quote.text),
attachments: (quote.attachments ?? []).map(attachment => {
return {
contentType: dropNull(attachment.contentType),
contentType: attachment.contentType
? stringToMIMEType(attachment.contentType)
: APPLICATION_OCTET_STREAM,
fileName: dropNull(attachment.fileName),
thumbnail: processAttachment(attachment.thumbnail),
};

View file

@ -185,12 +185,11 @@ export type AttachmentDraftType =
size: number;
};
export type ThumbnailType = Pick<
AttachmentType,
'height' | 'width' | 'url' | 'contentType' | 'path' | 'data'
> & {
export type ThumbnailType = AttachmentType & {
// Only used when quote needed to make an in-memory thumbnail
objectUrl?: string;
// Whether the thumbnail has been copied from the original (quoted) message
copied?: boolean;
};
// // Incoming message attachment fields
@ -452,6 +451,7 @@ export async function captureDimensionsAndScreenshot(
contentType: THUMBNAIL_CONTENT_TYPE,
width: THUMBNAIL_SIZE,
height: THUMBNAIL_SIZE,
size: 100,
},
};
} catch (error) {
@ -511,6 +511,7 @@ export async function captureDimensionsAndScreenshot(
contentType: THUMBNAIL_CONTENT_TYPE,
width: THUMBNAIL_SIZE,
height: THUMBNAIL_SIZE,
size: 100,
},
width,
height,
@ -873,7 +874,9 @@ export const isFile = (attachment: AttachmentType): boolean => {
return true;
};
export const isVoiceMessage = (attachment: AttachmentType): boolean => {
export const isVoiceMessage = (
attachment: Pick<AttachmentType, 'contentType' | 'fileName' | 'flags'>
): boolean => {
const flag = SignalService.AttachmentPointer.Flags.VOICE_MESSAGE;
const hasFlag =
// eslint-disable-next-line no-bitwise

View file

@ -25,6 +25,7 @@ import type {
import type {
MessageAttributesType,
QuotedAttachmentType,
QuotedMessageType,
} from '../model-types.d';
import type {
@ -309,8 +310,8 @@ export const _mapQuotedAttachments =
}
const upgradeWithContext = async (
attachment: AttachmentType
): Promise<AttachmentType> => {
attachment: QuotedAttachmentType
): Promise<QuotedAttachmentType> => {
const { thumbnail } = attachment;
if (!thumbnail) {
return attachment;
@ -995,7 +996,7 @@ export const createAttachmentDataWriter = ({
}
});
const writeQuoteAttachment = async (attachment: AttachmentType) => {
const writeQuoteAttachment = async (attachment: QuotedAttachmentType) => {
const { thumbnail } = attachment;
if (!thumbnail) {
return attachment;

View file

@ -6,6 +6,7 @@ import type { EditAttributesType } from '../messageModifiers/Edits';
import type {
EditHistoryType,
MessageAttributesType,
QuotedAttachmentType,
QuotedMessageType,
} from '../model-types.d';
import type { LinkPreviewType } from '../types/message/LinkPreviews';
@ -143,7 +144,7 @@ export async function handleEditMessage(
// and they have already been downloaded.
const attachmentSignatures: Map<string, AttachmentType> = new Map();
const previewSignatures: Map<string, LinkPreviewType> = new Map();
const quoteSignatures: Map<string, AttachmentType> = new Map();
const quoteSignatures: Map<string, QuotedAttachmentType> = new Map();
mainMessage.attachments?.forEach(attachment => {
const signature = getAttachmentSignatureSafe(attachment);
@ -226,13 +227,13 @@ export async function handleEditMessage(
return attachment;
}
const signature = getAttachmentSignatureSafe(attachment.thumbnail);
const existingThumbnail = signature
const existingQuoteAttachment = signature
? quoteSignatures.get(signature)
: undefined;
if (existingThumbnail) {
if (existingQuoteAttachment) {
return {
...attachment,
thumbnail: existingThumbnail,
thumbnail: existingQuoteAttachment.thumbnail,
};
}

View file

@ -1,12 +1,12 @@
// Copyright 2023 Signal Messenger, LLC
// SPDX-License-Identifier: AGPL-3.0-only
import type { AttachmentType, ThumbnailType } from '../types/Attachment';
import type { AttachmentType } from '../types/Attachment';
import type {
MessageAttributesType,
QuotedAttachmentType,
QuotedMessageType,
} from '../model-types.d';
import type { MIMEType } from '../types/MIME';
import type { LinkPreviewType } from '../types/message/LinkPreviews';
import type { StickerType } from '../types/Stickers';
import { IMAGE_JPEG, IMAGE_GIF } from '../types/MIME';
@ -40,7 +40,7 @@ export async function makeQuote(
return {
authorAci: contact.getCheckedAci('makeQuote'),
attachments: isTapToView(quotedMessage)
? [{ contentType: IMAGE_JPEG, fileName: null }]
? [{ contentType: IMAGE_JPEG }]
: await getQuoteAttachment(attachments, preview, sticker),
payment,
bodyRanges,
@ -57,13 +57,7 @@ export async function getQuoteAttachment(
attachments?: Array<AttachmentType>,
preview?: Array<LinkPreviewType>,
sticker?: StickerType
): Promise<
Array<{
contentType: MIMEType;
fileName?: string | null;
thumbnail?: ThumbnailType | null;
}>
> {
): Promise<Array<QuotedAttachmentType>> {
const { getAbsoluteAttachmentPath, loadAttachmentData } =
window.Signal.Migrations;
@ -78,18 +72,14 @@ export async function getQuoteAttachment(
if (!path) {
return {
contentType: isGIFQuote ? IMAGE_GIF : contentType,
// Our protos library complains about this field being undefined, so we
// force it to null
fileName: fileName || null,
thumbnail: null,
fileName,
thumbnail,
};
}
return {
contentType: isGIFQuote ? IMAGE_GIF : contentType,
// Our protos library complains about this field being undefined, so we force
// it to null
fileName: fileName || null,
fileName,
thumbnail: thumbnail
? {
...(await loadAttachmentData(thumbnail)),
@ -97,7 +87,7 @@ export async function getQuoteAttachment(
? getAbsoluteAttachmentPath(thumbnail.path)
: undefined,
}
: null,
: undefined,
};
})
);
@ -113,9 +103,6 @@ export async function getQuoteAttachment(
return {
contentType,
// Our protos library complains about this field being undefined, so we
// force it to null
fileName: null,
thumbnail: image
? {
...(await loadAttachmentData(image)),
@ -123,7 +110,7 @@ export async function getQuoteAttachment(
? getAbsoluteAttachmentPath(image.path)
: undefined,
}
: null,
: undefined,
};
})
);
@ -135,9 +122,6 @@ export async function getQuoteAttachment(
return [
{
contentType,
// Our protos library complains about this field being undefined, so we
// force it to null
fileName: null,
thumbnail: {
...(await loadAttachmentData(sticker.data)),
objectUrl: path ? getAbsoluteAttachmentPath(path) : undefined,

View file

@ -12,7 +12,7 @@ import {
} from '../types/Stickers';
import dataInterface from '../sql/Client';
import type { AttachmentType } from '../types/Attachment';
import type { AttachmentType, ThumbnailType } from '../types/Attachment';
import type { EmbeddedContactType } from '../types/EmbeddedContact';
import type {
EditHistoryType,
@ -540,17 +540,17 @@ async function queueQuoteAttachments({
// Similar to queueNormalAttachments' logic for detecting same attachments
// except here we also pick by quote sent timestamp.
const thumbnailSignatures: Map<string, AttachmentType> = new Map();
const thumbnailSignatures: Map<string, ThumbnailType> = new Map();
otherQuotes.forEach(otherQuote => {
for (const attachment of otherQuote.attachments) {
const signature = getQuoteThumbnailSignature(
otherQuote,
attachment.thumbnail
);
if (!signature) {
if (!signature || !attachment.thumbnail) {
continue;
}
thumbnailSignatures.set(signature, attachment);
thumbnailSignatures.set(signature, attachment.thumbnail);
}
});