Reuse calculated height & width when deduplicating attachments

This commit is contained in:
trevor-signal
2026-02-11 11:51:38 -05:00
committed by GitHub
parent 09b006e14b
commit 8d2706bf25
5 changed files with 20 additions and 5 deletions

View File

@@ -1059,7 +1059,13 @@ function _markAttachmentAsTransientlyErrored(
type AttachmentDataToBeReused = WithRequiredProperties<
Pick<
AttachmentType,
'path' | 'localKey' | 'version' | 'thumbnail' | 'screenshot'
| 'path'
| 'localKey'
| 'version'
| 'thumbnail'
| 'screenshot'
| 'width'
| 'height'
>,
'path' | 'localKey' | 'version'
>;
@@ -1107,6 +1113,8 @@ async function getExistingAttachmentDataForReuse({
path: existingAttachmentData.path,
localKey: existingAttachmentData.localKey,
version: existingAttachmentData.version,
width: existingAttachmentData.width ?? undefined,
height: existingAttachmentData.height ?? undefined,
};
const { thumbnailPath, thumbnailSize, thumbnailContentType } =
existingAttachmentData;

View File

@@ -809,6 +809,8 @@ export type ExistingAttachmentData = Pick<
| 'version'
| 'path'
| 'localKey'
| 'width'
| 'height'
| 'thumbnailPath'
| 'thumbnailLocalKey'
| 'thumbnailVersion'

View File

@@ -2951,6 +2951,8 @@ function getAndProtectExistingAttachmentPath(
path,
version,
localKey,
width,
height,
thumbnailPath,
thumbnailLocalKey,
thumbnailVersion,

View File

@@ -1228,6 +1228,8 @@ describe('AttachmentDownloadManager.runDownloadAttachmentJobInner', () => {
size: 128,
contentType: MIME.VIDEO_MP4,
version: 2,
width: 1123,
height: 5811,
plaintextHash: testPlaintextHash(),
path: 'existingPath',
localKey: testAttachmentLocalKey(),
@@ -1353,6 +1355,8 @@ describe('AttachmentDownloadManager.runDownloadAttachmentJobInner', () => {
'path',
'localKey',
'version',
'width',
'height',
'thumbnail.path',
'thumbnail.localKey',
'thumbnail.size',
@@ -1366,6 +1370,7 @@ describe('AttachmentDownloadManager.runDownloadAttachmentJobInner', () => {
];
assert.strictEqual(attachment?.path, existingAttachment.path);
assert.strictEqual(attachment?.width, existingAttachment.width);
assert.deepStrictEqual(
pick(attachment, propsThatShouldBeTransferred),
pick(existingAttachment, propsThatShouldBeTransferred)

View File

@@ -81,8 +81,7 @@ export async function captureDimensionsAndScreenshot(
const localUrl = getLocalAttachmentUrl(attachment);
if (isImageTypeSupported(contentType)) {
if (attachment.thumbnail?.path) {
// Already generated thumbnail / width / height
if (attachment.height && attachment.width && attachment.thumbnail?.path) {
return attachment;
}
@@ -133,8 +132,7 @@ export async function captureDimensionsAndScreenshot(
strictAssert(isVideoTypeSupported(contentType), 'enforced by early return');
if (attachment.screenshot?.path) {
// Already generated screenshot / width / height
if (attachment.height && attachment.width && attachment.screenshot?.path) {
return attachment;
}