From 1d2c3ae23c4a95004bb8489dda0ae1101a00441a Mon Sep 17 00:00:00 2001 From: Scott Nonnenberg Date: Wed, 30 Jan 2019 12:15:07 -0800 Subject: [PATCH] Download attachments in separate queue from message processing --- app/sql.js | 104 +++++ images/spinner-24.svg | 9 + images/spinner-56.svg | 9 + images/spinner-track-24.svg | 9 + images/spinner-track-56.svg | 9 + js/background.js | 16 +- js/models/messages.js | 140 +++++- js/modules/attachment_downloads.js | 410 ++++++++++++++++++ js/modules/data.js | 28 ++ js/modules/signal.js | 16 +- js/modules/types/attachment.js | 10 + .../attachment/migrate_data_to_file_system.js | 3 +- js/modules/types/message.js | 82 +++- js/views/conversation_view.js | 54 ++- libtextsecure/message_receiver.js | 169 ++++---- stylesheets/_ios.scss | 52 +++ stylesheets/_modules.scss | 94 +++- stylesheets/_theme_dark.scss | 49 +++ stylesheets/_variables.scss | 1 + test/modules/types/message_test.js | 16 +- ts/components/Spinner.md | 15 + ts/components/Spinner.tsx | 47 ++ ts/components/conversation/EmbeddedContact.md | 45 ++ .../conversation/EmbeddedContact.tsx | 15 +- ts/components/conversation/Image.md | 261 ++++++++++- ts/components/conversation/Image.tsx | 43 +- ts/components/conversation/ImageGrid.tsx | 6 +- ts/components/conversation/Message.md | 280 ++++++++++++ ts/components/conversation/Message.tsx | 41 +- ts/components/conversation/Quote.md | 44 ++ ts/components/conversation/Quote.tsx | 38 +- ts/components/conversation/types.ts | 1 + ts/test/types/Contact_test.ts | 134 +++++- ts/types/Contact.ts | 26 +- 34 files changed, 2062 insertions(+), 214 deletions(-) create mode 100644 images/spinner-24.svg create mode 100644 images/spinner-56.svg create mode 100644 images/spinner-track-24.svg create mode 100644 images/spinner-track-56.svg create mode 100644 js/modules/attachment_downloads.js create mode 100644 ts/components/Spinner.md create mode 100644 ts/components/Spinner.tsx diff --git a/app/sql.js b/app/sql.js index 52901f663a48..49472d73f3d8 100644 --- a/app/sql.js +++ b/app/sql.js @@ -96,6 +96,13 @@ module.exports = { removeUnprocessed, removeAllUnprocessed, + getNextAttachmentDownloadJobs, + saveAttachmentDownloadJob, + setAttachmentDownloadJobPending, + resetAttachmentDownloadPending, + removeAttachmentDownloadJob, + removeAllAttachmentDownloadJobs, + removeAll, removeAllConfiguration, @@ -525,6 +532,34 @@ async function updateToSchemaVersion8(currentVersion, instance) { console.log('updateToSchemaVersion8: success!'); } +async function updateToSchemaVersion9(currentVersion, instance) { + if (currentVersion >= 9) { + return; + } + console.log('updateToSchemaVersion9: starting...'); + await instance.run('BEGIN TRANSACTION;'); + + await instance.run(`CREATE TABLE attachment_downloads( + id STRING primary key, + timestamp INTEGER, + pending INTEGER, + json TEXT + );`); + + await instance.run(`CREATE INDEX attachment_downloads_timestamp + ON attachment_downloads ( + timestamp + ) WHERE pending = 0;`); + await instance.run(`CREATE INDEX attachment_downloads_pending + ON attachment_downloads ( + pending + ) WHERE pending != 0;`); + + await instance.run('PRAGMA schema_version = 9;'); + await instance.run('COMMIT TRANSACTION;'); + console.log('updateToSchemaVersion9: success!'); +} + const SCHEMA_VERSIONS = [ updateToSchemaVersion1, updateToSchemaVersion2, @@ -534,6 +569,7 @@ const SCHEMA_VERSIONS = [ updateToSchemaVersion6, updateToSchemaVersion7, updateToSchemaVersion8, + updateToSchemaVersion9, ]; async function updateSchema(instance) { @@ -1476,6 +1512,72 @@ async function removeAllUnprocessed() { await db.run('DELETE FROM unprocessed;'); } +const ATTACHMENT_DOWNLOADS_TABLE = 'attachment_downloads'; +async function getNextAttachmentDownloadJobs(limit, options = {}) { + const timestamp = options.timestamp || Date.now(); + + const rows = await db.all( + `SELECT json FROM attachment_downloads + WHERE pending = 0 AND timestamp < $timestamp + ORDER BY timestamp DESC + LIMIT $limit;`, + { + $limit: limit, + $timestamp: timestamp, + } + ); + + return map(rows, row => jsonToObject(row.json)); +} +async function saveAttachmentDownloadJob(job) { + const { id, pending, timestamp } = job; + if (!id) { + throw new Error( + 'saveAttachmentDownloadJob: Provided job did not have a truthy id' + ); + } + + await db.run( + `INSERT OR REPLACE INTO attachment_downloads ( + id, + pending, + timestamp, + json + ) values ( + $id, + $pending, + $timestamp, + $json + )`, + { + $id: id, + $pending: pending, + $timestamp: timestamp, + $json: objectToJSON(job), + } + ); +} +async function setAttachmentDownloadJobPending(id, pending) { + await db.run( + 'UPDATE attachment_downloads SET pending = $pending WHERE id = $id;', + { + $id: id, + $pending: pending, + } + ); +} +async function resetAttachmentDownloadPending() { + await db.run( + 'UPDATE attachment_downloads SET pending = 0 WHERE pending != 0;' + ); +} +async function removeAttachmentDownloadJob(id) { + return removeById(ATTACHMENT_DOWNLOADS_TABLE, id); +} +async function removeAllAttachmentDownloadJobs() { + return removeAllFromTable(ATTACHMENT_DOWNLOADS_TABLE); +} + // All data in database async function removeAll() { let promise; @@ -1492,6 +1594,8 @@ async function removeAll() { db.run('DELETE FROM sessions;'), db.run('DELETE FROM signedPreKeys;'), db.run('DELETE FROM unprocessed;'), + db.run('DELETE FROM attachment_downloads;'), + db.run('DELETE FROM messages_fts;'), db.run('COMMIT TRANSACTION;'), ]); }); diff --git a/images/spinner-24.svg b/images/spinner-24.svg new file mode 100644 index 000000000000..392aa714f334 --- /dev/null +++ b/images/spinner-24.svg @@ -0,0 +1,9 @@ + + + + Interderminate Spinner - 24 + Created with Sketch. + + + + \ No newline at end of file diff --git a/images/spinner-56.svg b/images/spinner-56.svg new file mode 100644 index 000000000000..0badeb1095cd --- /dev/null +++ b/images/spinner-56.svg @@ -0,0 +1,9 @@ + + + + Interderminate Spinner - 56 + Created with Sketch. + + + + \ No newline at end of file diff --git a/images/spinner-track-24.svg b/images/spinner-track-24.svg new file mode 100644 index 000000000000..0ab7cac2abf6 --- /dev/null +++ b/images/spinner-track-24.svg @@ -0,0 +1,9 @@ + + + + Interderminate Track - 24 + Created with Sketch. + + + + \ No newline at end of file diff --git a/images/spinner-track-56.svg b/images/spinner-track-56.svg new file mode 100644 index 000000000000..d0aad7dac495 --- /dev/null +++ b/images/spinner-track-56.svg @@ -0,0 +1,9 @@ + + + + Interderminate Track - 56 + Created with Sketch. + + + + \ No newline at end of file diff --git a/js/background.js b/js/background.js index f3a5c680b506..f1000f0ce6d3 100644 --- a/js/background.js +++ b/js/background.js @@ -602,6 +602,7 @@ if (messageReceiver) { messageReceiver.close(); } + window.Signal.AttachmentDownloads.stop(); } let connectCount = 0; @@ -666,6 +667,11 @@ messageReceiver.addEventListener('configuration', onConfiguration); messageReceiver.addEventListener('typing', onTyping); + window.Signal.AttachmentDownloads.start({ + getMessageReceiver: () => messageReceiver, + logger: window.log, + }); + window.textsecure.messaging = new textsecure.MessageSender( USERNAME, PASSWORD @@ -1138,7 +1144,10 @@ const { thumbnail } = queryFirst; if (thumbnail && thumbnail.path) { - firstAttachment.thumbnail = thumbnail; + firstAttachment.thumbnail = { + ...thumbnail, + copied: true, + }; } } @@ -1148,7 +1157,10 @@ const { image } = queryFirst; if (image && image.path) { - firstAttachment.thumbnail = image; + firstAttachment.thumbnail = { + ...image, + copied: true, + }; } } diff --git a/js/models/messages.js b/js/models/messages.js index 9b342a076f1a..1da77e20a4e6 100644 --- a/js/models/messages.js +++ b/js/models/messages.js @@ -19,13 +19,11 @@ const { Message: TypedMessage, Contact, PhoneNumber } = Signal.Types; const { - deleteAttachmentData, deleteExternalMessageFiles, getAbsoluteAttachmentPath, loadAttachmentData, loadQuoteData, loadPreviewData, - writeNewAttachmentData, } = window.Signal.Migrations; window.AccountCache = Object.create(null); @@ -423,9 +421,9 @@ authorProfileName: contact.profileName, authorPhoneNumber: contact.phoneNumber, conversationType: isGroup ? 'group' : 'direct', - attachments: attachments.map(attachment => - this.getPropsForAttachment(attachment) - ), + attachments: attachments + .filter(attachment => !attachment.error) + .map(attachment => this.getPropsForAttachment(attachment)), previews: this.getPropsForPreview(), quote: this.getPropsForQuote(), authorAvatarPath, @@ -586,7 +584,7 @@ return null; } - const { path, flags, size, screenshot, thumbnail } = attachment; + const { path, pending, flags, size, screenshot, thumbnail } = attachment; return { ...attachment, @@ -595,7 +593,8 @@ flags && // eslint-disable-next-line no-bitwise flags & textsecure.protobuf.AttachmentPointer.Flags.VOICE_MESSAGE, - url: getAbsoluteAttachmentPath(path), + pending, + url: path ? getAbsoluteAttachmentPath(path) : null, screenshot: screenshot ? { ...screenshot, @@ -1155,6 +1154,116 @@ ); return !!error; }, + async queueAttachmentDownloads() { + const messageId = this.id; + let count = 0; + + const attachments = await Promise.all( + (this.get('attachments') || []).map((attachment, index) => { + count += 1; + return window.Signal.AttachmentDownloads.addJob(attachment, { + messageId, + type: 'attachment', + index, + }); + }) + ); + + const preview = await Promise.all( + (this.get('preview') || []).map(async (item, index) => { + if (!item.image) { + return item; + } + + count += 1; + return { + ...item, + image: await window.Signal.AttachmentDownloads.addJob(item.image, { + messageId, + type: 'preview', + index, + }), + }; + }) + ); + + const contact = await Promise.all( + (this.get('contact') || []).map(async (item, index) => { + if (!item.avatar || !item.avatar.avatar) { + return item; + } + + count += 1; + return { + ...item, + avatar: { + ...item.avatar, + avatar: await window.Signal.AttachmentDownloads.addJob( + item.avatar.avatar, + { + messageId, + type: 'contact', + index, + } + ), + }, + }; + }) + ); + + let quote = this.get('quote'); + if (quote && quote.attachments && quote.attachments.length) { + quote = { + ...quote, + attachments: await Promise.all( + (quote.attachments || []).map(async (item, index) => { + // If we already have a path, then we copied this image from the quoted + // message and we don't need to download the attachment. + if (!item.thumbnail || item.thumbnail.path) { + return item; + } + + count += 1; + return { + ...item, + thumbnail: await window.Signal.AttachmentDownloads.addJob( + item.thumbnail, + { + messageId, + type: 'quote', + index, + } + ), + }; + }) + ), + }; + } + + let group = this.get('group'); + if (group && group.avatar) { + group = { + ...group, + avatar: await window.Signal.AttachmentDownloads.addJob(group.avatar, { + messageId, + type: 'group-avatar', + index: 0, + }), + }; + } + + if (count > 0) { + this.set({ attachments, preview, contact, quote, group }); + + await window.Signal.Data.saveMessage(this.attributes, { + Message: Whisper.Message, + }); + + return true; + } + + return false; + }, handleDataMessage(dataMessage, confirm) { // This function is called from the background script in a few scenarios: // 1. on an incoming message @@ -1194,18 +1303,6 @@ ), }; - // Update this group conversations's avatar on disk if it has changed. - if (dataMessage.group.avatar) { - attributes = await window.Signal.Types.Conversation.maybeUpdateAvatar( - attributes, - dataMessage.group.avatar.data, - { - writeNewAttachmentData, - deleteAttachmentData, - } - ); - } - groupUpdate = conversation.changedAttributes( _.pick(dataMessage.group, 'name', 'avatar') @@ -1420,6 +1517,11 @@ }); message.set({ id }); + // Note that this can save the message again, if jobs were queued. We need to + // call it after we have an id for this message, because the jobs refer back + // to their source message. + await message.queueAttachmentDownloads(); + await window.Signal.Data.updateConversation( conversationId, conversation.attributes, diff --git a/js/modules/attachment_downloads.js b/js/modules/attachment_downloads.js new file mode 100644 index 000000000000..60136b67b603 --- /dev/null +++ b/js/modules/attachment_downloads.js @@ -0,0 +1,410 @@ +/* global Whisper, Signal, setTimeout, clearTimeout */ + +const { isFunction, isNumber, omit } = require('lodash'); +const getGuid = require('uuid/v4'); +const { + getMessageById, + getNextAttachmentDownloadJobs, + removeAttachmentDownloadJob, + resetAttachmentDownloadPending, + saveAttachmentDownloadJob, + saveMessage, + setAttachmentDownloadJobPending, +} = require('./data'); + +module.exports = { + start, + stop, + addJob, +}; + +const MAX_ATTACHMENT_JOB_PARALLELISM = 3; + +const SECOND = 1000; +const MINUTE = 60 * SECOND; +const HOUR = 60 * MINUTE; +const TICK_INTERVAL = MINUTE; + +const RETRY_BACKOFF = { + 1: 30 * SECOND, + 2: 30 * MINUTE, + 3: 6 * HOUR, +}; + +let enabled = false; +let timeout; +let getMessageReceiver; +let logger; +const _activeAttachmentDownloadJobs = {}; +const _messageCache = {}; + +async function start(options = {}) { + ({ getMessageReceiver, logger } = options); + if (!isFunction(getMessageReceiver)) { + throw new Error( + 'attachment_downloads/start: getMessageReceiver must be a function' + ); + } + if (!logger) { + throw new Error('attachment_downloads/start: logger must be provided!'); + } + + enabled = true; + await resetAttachmentDownloadPending(); + + _tick(); +} + +async function stop() { + enabled = false; + if (timeout) { + clearTimeout(timeout); + timeout = null; + } +} + +async function addJob(attachment, job = {}) { + if (!attachment) { + throw new Error('attachments_download/addJob: attachment is required'); + } + + const { messageId, type, index } = job; + if (!messageId) { + throw new Error('attachments_download/addJob: job.messageId is required'); + } + if (!type) { + throw new Error('attachments_download/addJob: job.type is required'); + } + if (!isNumber(index)) { + throw new Error('attachments_download/addJob: index must be a number'); + } + + const id = getGuid(); + const timestamp = Date.now(); + const toSave = { + ...job, + id, + attachment, + timestamp, + pending: 0, + attempts: 0, + }; + + await saveAttachmentDownloadJob(toSave); + + _maybeStartJob(); + + return { + ...attachment, + pending: true, + downloadJobId: id, + }; +} + +async function _tick() { + _maybeStartJob(); + timeout = setTimeout(_tick, TICK_INTERVAL); +} + +async function _maybeStartJob() { + if (!enabled) { + return; + } + + const jobCount = getActiveJobCount(); + const limit = MAX_ATTACHMENT_JOB_PARALLELISM - jobCount; + if (limit <= 0) { + return; + } + + const nextJobs = await getNextAttachmentDownloadJobs(limit); + if (nextJobs.length <= 0) { + return; + } + + // To prevent the race condition caused by two parallel database calls, eached kicked + // off because the jobCount wasn't at the max. + const secondJobCount = getActiveJobCount(); + const needed = MAX_ATTACHMENT_JOB_PARALLELISM - secondJobCount; + if (needed <= 0) { + return; + } + + const jobs = nextJobs.slice(0, Math.min(needed, nextJobs.length)); + for (let i = 0, max = jobs.length; i < max; i += 1) { + const job = jobs[i]; + _activeAttachmentDownloadJobs[job.id] = _runJob(job); + } +} + +async function _runJob(job) { + const { id, messageId, attachment, type, index, attempts } = job || {}; + let message; + + try { + if (!job || !attachment || !messageId) { + throw new Error( + `_runJob: Key information required for job was missing. Job id: ${id}` + ); + } + + message = await _getMessage(messageId); + if (!message) { + logger.error('_runJob: Source message not found, deleting job'); + await _finishJob(message, id); + return; + } + + const pending = true; + await setAttachmentDownloadJobPending(id, pending); + + let downloaded; + const messageReceiver = getMessageReceiver(); + if (!messageReceiver) { + throw new Error('_runJob: messageReceiver not found'); + } + + try { + downloaded = await messageReceiver.downloadAttachment(attachment); + } catch (error) { + // Attachments on the server expire after 30 days, then start returning 404 + if (error && error.code === 404) { + logger.warn( + `_runJob: Got 404 from server, marking attachment ${ + attachment.id + } from message ${message.idForLogging()} as permanent error` + ); + + await _finishJob(message, id); + await _addAttachmentToMessage( + message, + _markAttachmentAsError(attachment), + { type, index } + ); + + return; + } + throw error; + } + + const upgradedAttachment = await Signal.Migrations.processNewAttachment( + downloaded + ); + + await _addAttachmentToMessage(message, upgradedAttachment, { type, index }); + + await _finishJob(message, id); + } catch (error) { + const currentAttempt = (attempts || 0) + 1; + + if (currentAttempt >= 3) { + logger.error( + `_runJob: ${currentAttempt} failed attempts, marking attachment ${id} from message ${message.idForLogging()} as permament error:`, + error && error.stack ? error.stack : error + ); + + await _finishJob(message, id); + await _addAttachmentToMessage( + message, + _markAttachmentAsError(attachment), + { type, index } + ); + + return; + } + + logger.error( + `_runJob: Failed to download attachment type ${type} for message ${message.idForLogging()}, attempt ${currentAttempt}:`, + error && error.stack ? error.stack : error + ); + + const failedJob = { + ...job, + pending: 0, + attempts: currentAttempt, + timestamp: Date.now() + RETRY_BACKOFF[currentAttempt], + }; + + await saveAttachmentDownloadJob(failedJob); + delete _activeAttachmentDownloadJobs[id]; + _maybeStartJob(); + } +} + +async function _getMessage(id) { + let item = _messageCache[id]; + if (item) { + const fiveMinutesAgo = Date.now() - 5 * MINUTE; + if (item.timestamp >= fiveMinutesAgo) { + return item.message; + } + + delete _messageCache[id]; + } + + let message = await getMessageById(id, { + Message: Whisper.Message, + }); + if (!message) { + return message; + } + + // Once more, checking for race conditions + item = _messageCache[id]; + if (item) { + const fiveMinutesAgo = Date.now() - 5 * MINUTE; + if (item.timestamp >= fiveMinutesAgo) { + return item.message; + } + } + + const conversation = message.getConversation(); + if (conversation && conversation.messageCollection.get(id)) { + message = conversation.get(id); + } + + _messageCache[id] = { + timestamp: Date.now(), + message, + }; + + return message; +} + +async function _finishJob(message, id) { + if (message) { + await saveMessage(message.attributes, { + Message: Whisper.Message, + }); + const conversation = message.getConversation(); + if (conversation) { + const fromConversation = conversation.messageCollection.get(message.id); + + if (fromConversation && message !== fromConversation) { + fromConversation.set(message.attributes); + fromConversation.trigger('change'); + } else { + message.trigger('change'); + } + } + } + + await removeAttachmentDownloadJob(id); + delete _activeAttachmentDownloadJobs[id]; + _maybeStartJob(); +} + +function getActiveJobCount() { + return Object.keys(_activeAttachmentDownloadJobs).length; +} + +function _markAttachmentAsError(attachment) { + return { + ...omit(attachment, ['key', 'digest', 'id']), + error: true, + }; +} + +async function _addAttachmentToMessage(message, attachment, { type, index }) { + if (!message) { + return; + } + + if (type === 'attachment') { + const attachments = message.get('attachments'); + if (!attachments || attachments.length <= index) { + throw new Error( + `_addAttachmentToMessage: attachments didn't exist or ${index} was too large` + ); + } + _replaceAttachment(attachments, index, attachment); + return; + } + + if (type === 'preview') { + const preview = message.get('preview'); + if (!preview || preview.length <= index) { + throw new Error( + `_addAttachmentToMessage: preview didn't exist or ${index} was too large` + ); + } + const item = preview[index]; + if (!item) { + throw new Error(`_addAttachmentToMessage: preview ${index} was falsey`); + } + _replaceAttachment(item, 'image', attachment); + return; + } + + if (type === 'contact') { + const contact = message.get('contact'); + if (!contact || contact.length <= index) { + throw new Error( + `_addAttachmentToMessage: contact didn't exist or ${index} was too large` + ); + } + const item = contact[index]; + if (item && item.avatar && item.avatar.avatar) { + _replaceAttachment(item.avatar, 'avatar', attachment); + } else { + logger.warn( + `_addAttachmentToMessage: Couldn't update contact with avatar attachment for message ${message.idForLogging()}` + ); + } + + return; + } + + if (type === 'quote') { + const quote = message.get('quote'); + if (!quote) { + throw new Error("_addAttachmentToMessage: quote didn't exist"); + } + const { attachments } = quote; + if (!attachments || attachments.length <= index) { + throw new Error( + `_addAttachmentToMessage: quote attachments didn't exist or ${index} was too large` + ); + } + + const item = attachments[index]; + if (!item) { + throw new Error( + `_addAttachmentToMessage: attachment ${index} was falsey` + ); + } + _replaceAttachment(item, 'thumbnail', attachment); + return; + } + + if (type === 'group-avatar') { + const group = message.get('group'); + if (!group) { + throw new Error("_addAttachmentToMessage: group didn't exist"); + } + + const existingAvatar = group.avatar; + if (existingAvatar && existingAvatar.path) { + await Signal.Migrations.deleteAttachmentData(existingAvatar.path); + } + + _replaceAttachment(group, 'avatar', attachment); + return; + } + + throw new Error( + `_addAttachmentToMessage: Unknown job type ${type} for message ${message.idForLogging()}` + ); +} + +function _replaceAttachment(object, key, newAttachment) { + const oldAttachment = object[key]; + if (oldAttachment && oldAttachment.path) { + logger.warn( + '_replaceAttachment: Old attachment already had path, not replacing' + ); + } + + // eslint-disable-next-line no-param-reassign + object[key] = newAttachment; +} diff --git a/js/modules/data.js b/js/modules/data.js index 83cfa1153282..debfec629219 100644 --- a/js/modules/data.js +++ b/js/modules/data.js @@ -131,6 +131,13 @@ module.exports = { removeUnprocessed, removeAllUnprocessed, + getNextAttachmentDownloadJobs, + saveAttachmentDownloadJob, + resetAttachmentDownloadPending, + setAttachmentDownloadJobPending, + removeAttachmentDownloadJob, + removeAllAttachmentDownloadJobs, + removeAll, removeAllConfiguration, @@ -854,6 +861,27 @@ async function removeAllUnprocessed() { await channels.removeAllUnprocessed(); } +// Attachment downloads + +async function getNextAttachmentDownloadJobs(limit) { + return channels.getNextAttachmentDownloadJobs(limit); +} +async function saveAttachmentDownloadJob(job) { + await channels.saveAttachmentDownloadJob(job); +} +async function setAttachmentDownloadJobPending(id, pending) { + await channels.setAttachmentDownloadJobPending(id, pending); +} +async function resetAttachmentDownloadPending() { + await channels.resetAttachmentDownloadPending(); +} +async function removeAttachmentDownloadJob(id) { + await channels.removeAttachmentDownloadJob(id); +} +async function removeAllAttachmentDownloadJobs() { + await channels.removeAllAttachmentDownloadJobs(); +} + // Other async function removeAll() { diff --git a/js/modules/signal.js b/js/modules/signal.js index ded0e63ba066..e51ecd72112a 100644 --- a/js/modules/signal.js +++ b/js/modules/signal.js @@ -14,6 +14,7 @@ const { migrateToSQL } = require('./migrate_to_sql'); const Metadata = require('./metadata/SecretSessionCipher'); const RefreshSenderCertificate = require('./refresh_sender_certificate'); const LinkPreviews = require('./link_previews'); +const AttachmentDownloads = require('./attachment_downloads'); // Components const { @@ -128,6 +129,7 @@ function initializeMigrations({ const loadQuoteData = MessageType.loadQuoteData(loadAttachmentData); const getAbsoluteAttachmentPath = createAbsolutePathGetter(attachmentsPath); const deleteOnDisk = Attachments.createDeleter(attachmentsPath); + const writeNewAttachmentData = createWriterForNew(attachmentsPath); return { attachmentsPath, @@ -145,11 +147,22 @@ function initializeMigrations({ loadQuoteData, readAttachmentData, run, + processNewAttachment: attachment => + MessageType.processNewAttachment(attachment, { + writeNewAttachmentData, + getAbsoluteAttachmentPath, + makeObjectUrl, + revokeObjectUrl, + getImageDimensions, + makeImageThumbnail, + makeVideoScreenshot, + logger, + }), upgradeMessageSchema: (message, options = {}) => { const { maxVersion } = options; return MessageType.upgradeSchema(message, { - writeNewAttachmentData: createWriterForNew(attachmentsPath), + writeNewAttachmentData, getRegionCode, getAbsoluteAttachmentPath, makeObjectUrl, @@ -233,6 +246,7 @@ exports.setup = (options = {}) => { }; return { + AttachmentDownloads, Backbone, Components, Crypto, diff --git a/js/modules/types/attachment.js b/js/modules/types/attachment.js index 272c0d43956a..d134af13ae21 100644 --- a/js/modules/types/attachment.js +++ b/js/modules/types/attachment.js @@ -56,6 +56,11 @@ exports.autoOrientJPEG = async attachment => { return attachment; } + // If we haven't downloaded the attachment yet, we won't have the data + if (!attachment.data) { + return attachment; + } + const dataBlob = await arrayBufferToBlob( attachment.data, attachment.contentType @@ -234,6 +239,11 @@ exports.captureDimensionsAndScreenshot = async ( return attachment; } + // If the attachment hasn't been downloaded yet, we won't have a path + if (!attachment.path) { + return attachment; + } + const absolutePath = await getAbsoluteAttachmentPath(attachment.path); if (GoogleChrome.isImageTypeSupported(contentType)) { diff --git a/js/modules/types/attachment/migrate_data_to_file_system.js b/js/modules/types/attachment/migrate_data_to_file_system.js index 83588587260e..dded244ad3d8 100644 --- a/js/modules/types/attachment/migrate_data_to_file_system.js +++ b/js/modules/types/attachment/migrate_data_to_file_system.js @@ -9,7 +9,7 @@ const { isArrayBuffer, isFunction, isUndefined, omit } = require('lodash'); // Promise Attachment exports.migrateDataToFileSystem = async ( attachment, - { writeNewAttachmentData, logger } = {} + { writeNewAttachmentData } = {} ) => { if (!isFunction(writeNewAttachmentData)) { throw new TypeError("'writeNewAttachmentData' must be a function"); @@ -19,7 +19,6 @@ exports.migrateDataToFileSystem = async ( const hasData = !isUndefined(data); const shouldSkipSchemaUpgrade = !hasData; if (shouldSkipSchemaUpgrade) { - logger.warn('WARNING: `attachment.data` is `undefined`'); return attachment; } diff --git a/js/modules/types/message.js b/js/modules/types/message.js index 405092b37c9d..382f899dbf79 100644 --- a/js/modules/types/message.js +++ b/js/modules/types/message.js @@ -134,8 +134,7 @@ exports._withSchemaVersion = ({ schemaVersion, upgrade }) => { logger.warn( 'WARNING: Message._withSchemaVersion: Unexpected version:', `Expected message to have version ${expectedVersion},`, - `but got ${message.schemaVersion}.`, - message + `but got ${message.schemaVersion}.` ); return message; } @@ -203,7 +202,6 @@ exports._mapQuotedAttachments = upgradeAttachment => async ( if (!context || !isObject(context.logger)) { throw new Error('_mapQuotedAttachments: context must have logger object'); } - const { logger } = context; const upgradeWithContext = async attachment => { const { thumbnail } = attachment; @@ -211,11 +209,6 @@ exports._mapQuotedAttachments = upgradeAttachment => async ( return attachment; } - if (!thumbnail.data && !thumbnail.path) { - logger.warn('Quoted attachment did not have thumbnail data; removing it'); - return omit(attachment, ['thumbnail']); - } - const upgradedThumbnail = await upgradeAttachment(thumbnail, context); return Object.assign({}, attachment, { thumbnail: upgradedThumbnail, @@ -247,7 +240,6 @@ exports._mapPreviewAttachments = upgradeAttachment => async ( if (!context || !isObject(context.logger)) { throw new Error('_mapPreviewAttachments: context must have logger object'); } - const { logger } = context; const upgradeWithContext = async preview => { const { image } = preview; @@ -255,11 +247,6 @@ exports._mapPreviewAttachments = upgradeAttachment => async ( return preview; } - if (!image.data && !image.path) { - logger.warn('Preview did not have image data; removing it'); - return omit(preview, ['image']); - } - const upgradedImage = await upgradeAttachment(image, context); return Object.assign({}, preview, { image: upgradedImage, @@ -413,6 +400,68 @@ exports.upgradeSchema = async ( return message; }; +// Runs on attachments outside of the schema upgrade process, since attachments are +// downloaded out of band. +exports.processNewAttachment = async ( + attachment, + { + writeNewAttachmentData, + getAbsoluteAttachmentPath, + makeObjectUrl, + revokeObjectUrl, + getImageDimensions, + makeImageThumbnail, + makeVideoScreenshot, + logger, + } = {} +) => { + if (!isFunction(writeNewAttachmentData)) { + throw new TypeError('context.writeNewAttachmentData is required'); + } + if (!isFunction(getAbsoluteAttachmentPath)) { + throw new TypeError('context.getAbsoluteAttachmentPath is required'); + } + if (!isFunction(makeObjectUrl)) { + throw new TypeError('context.makeObjectUrl is required'); + } + if (!isFunction(revokeObjectUrl)) { + throw new TypeError('context.revokeObjectUrl is required'); + } + if (!isFunction(getImageDimensions)) { + throw new TypeError('context.getImageDimensions is required'); + } + if (!isFunction(makeImageThumbnail)) { + throw new TypeError('context.makeImageThumbnail is required'); + } + if (!isFunction(makeVideoScreenshot)) { + throw new TypeError('context.makeVideoScreenshot is required'); + } + if (!isObject(logger)) { + throw new TypeError('context.logger is required'); + } + + const rotatedAttachment = await Attachment.autoOrientJPEG(attachment); + const onDiskAttachment = await Attachment.migrateDataToFileSystem( + rotatedAttachment, + { writeNewAttachmentData } + ); + const finalAttachment = await Attachment.captureDimensionsAndScreenshot( + onDiskAttachment, + { + writeNewAttachmentData, + getAbsoluteAttachmentPath, + makeObjectUrl, + revokeObjectUrl, + getImageDimensions, + makeImageThumbnail, + makeVideoScreenshot, + logger, + } + ); + + return finalAttachment; +}; + exports.createAttachmentLoader = loadAttachmentData => { if (!isFunction(loadAttachmentData)) { throw new TypeError( @@ -508,7 +557,10 @@ exports.deleteAllExternalFiles = ({ deleteAttachmentData, deleteOnDisk }) => { quote.attachments.map(async attachment => { const { thumbnail } = attachment; - if (thumbnail && thumbnail.path) { + // To prevent spoofing, we copy the original image from the quoted message. + // If so, it will have a 'copied' field. We don't want to delete it if it has + // that field set to true. + if (thumbnail && thumbnail.path && !thumbnail.copied) { await deleteOnDisk(thumbnail.path); } }) diff --git a/js/views/conversation_view.js b/js/views/conversation_view.js index 477b8fa48763..cfa769aeb84d 100644 --- a/js/views/conversation_view.js +++ b/js/views/conversation_view.js @@ -801,20 +801,25 @@ const media = _.flatten( rawMedia.map(message => { const { attachments } = message; - return (attachments || []).map((attachment, index) => { - const { thumbnail } = attachment; + return (attachments || []) + .filter( + attachment => + attachment.thumbnail && !attachment.pending && !attachment.error + ) + .map((attachment, index) => { + const { thumbnail } = attachment; - return { - objectURL: getAbsoluteAttachmentPath(attachment.path), - thumbnailObjectUrl: thumbnail - ? getAbsoluteAttachmentPath(thumbnail.path) - : null, - contentType: attachment.contentType, - index, - attachment, - message, - }; - }); + return { + objectURL: getAbsoluteAttachmentPath(attachment.path), + thumbnailObjectUrl: thumbnail + ? getAbsoluteAttachmentPath(thumbnail.path) + : null, + contentType: attachment.contentType, + index, + attachment, + message, + }; + }); }) ); @@ -1240,7 +1245,19 @@ } const attachments = message.get('attachments') || []; - if (attachments.length === 1) { + + const media = attachments + .filter(item => item.thumbnail && !item.pending && !item.error) + .map((item, index) => ({ + objectURL: getAbsoluteAttachmentPath(item.path), + path: item.path, + contentType: item.contentType, + index, + message, + attachment: item, + })); + + if (media.length === 1) { const props = { objectURL: getAbsoluteAttachmentPath(path), contentType, @@ -1258,16 +1275,9 @@ } const selectedIndex = _.findIndex( - attachments, + media, item => attachment.path === item.path ); - const media = attachments.map((item, index) => ({ - objectURL: getAbsoluteAttachmentPath(item.path), - contentType: item.contentType, - index, - message, - attachment: item, - })); const onSave = async (options = {}) => { Signal.Types.Attachment.save({ diff --git a/libtextsecure/message_receiver.js b/libtextsecure/message_receiver.js index dc13f649df6f..2bf13f364435 100644 --- a/libtextsecure/message_receiver.js +++ b/libtextsecure/message_receiver.js @@ -1118,8 +1118,11 @@ MessageReceiver.prototype.extend({ }, handleContacts(envelope, contacts) { window.log.info('contact sync'); - const attachmentPointer = contacts.blob; - return this.handleAttachment(attachmentPointer).then(() => { + const { blob } = contacts; + + // Note: we do not return here because we don't want to block the next message on + // this attachment download and a lot of processing of that attachment. + this.handleAttachment(blob).then(attachmentPointer => { const results = []; const contactBuffer = new ContactBuffer(attachmentPointer.data); let contactDetails = contactBuffer.next(); @@ -1142,8 +1145,11 @@ MessageReceiver.prototype.extend({ }, handleGroups(envelope, groups) { window.log.info('group sync'); - const attachmentPointer = groups.blob; - return this.handleAttachment(attachmentPointer).then(() => { + const { blob } = groups; + + // Note: we do not return here because we don't want to block the next message on + // this attachment download and a lot of processing of that attachment. + this.handleAttachment(blob).then(attachmentPointer => { const groupBuffer = new GroupBuffer(attachmentPointer.data); let groupDetails = groupBuffer.next(); const promises = []; @@ -1211,32 +1217,32 @@ MessageReceiver.prototype.extend({ isGroupBlocked(groupId) { return textsecure.storage.get('blocked-groups', []).indexOf(groupId) >= 0; }, + cleanAttachment(attachment) { + return { + ..._.omit(attachment, 'thumbnail'), + id: attachment.id.toString(), + key: attachment.key ? attachment.key.toString('base64') : null, + digest: attachment.digest ? attachment.digest.toString('base64') : null, + }; + }, + async downloadAttachment(attachment) { + const encrypted = await this.server.getAttachment(attachment.id); + const { key, digest } = attachment; + + const data = await textsecure.crypto.decryptAttachment( + encrypted, + window.Signal.Crypto.base64ToArrayBuffer(key), + window.Signal.Crypto.base64ToArrayBuffer(digest) + ); + + return { + ..._.omit(attachment, 'digest', 'key'), + data, + }; + }, handleAttachment(attachment) { - // eslint-disable-next-line no-param-reassign - attachment.id = attachment.id.toString(); - // eslint-disable-next-line no-param-reassign - attachment.key = attachment.key.toArrayBuffer(); - if (attachment.digest) { - // eslint-disable-next-line no-param-reassign - attachment.digest = attachment.digest.toArrayBuffer(); - } - function decryptAttachment(encrypted) { - return textsecure.crypto.decryptAttachment( - encrypted, - attachment.key, - attachment.digest - ); - } - - function updateAttachment(data) { - // eslint-disable-next-line no-param-reassign - attachment.data = data; - } - - return this.server - .getAttachment(attachment.id) - .then(decryptAttachment) - .then(updateAttachment); + const cleaned = this.cleanAttachment(attachment); + return this.downloadAttachment(cleaned); }, async handleEndSession(number) { window.log.info('got end session'); @@ -1291,14 +1297,6 @@ MessageReceiver.prototype.extend({ if (decrypted.group !== null) { decrypted.group.id = decrypted.group.id.toBinary(); - if ( - decrypted.group.type === textsecure.protobuf.GroupContext.Type.UPDATE - ) { - if (decrypted.group.avatar !== null) { - promises.push(this.handleAttachment(decrypted.group.avatar)); - } - } - const storageGroups = textsecure.storage.groups; promises.push( @@ -1366,65 +1364,67 @@ MessageReceiver.prototype.extend({ ); } - for (let i = 0; i < attachmentCount; i += 1) { - const attachment = decrypted.attachments[i]; - promises.push(this.handleAttachment(attachment)); - } + // Here we go from binary to string/base64 in all AttachmentPointer digest/key fields - const previewCount = (decrypted.preview || []).length; - for (let i = 0; i < previewCount; i += 1) { - const preview = decrypted.preview[i]; - if (preview.image) { - promises.push(this.handleAttachment(preview.image)); + if ( + decrypted.group && + decrypted.group.type === textsecure.protobuf.GroupContext.Type.UPDATE + ) { + if (decrypted.group.avatar !== null) { + decrypted.group.avatar = this.cleanAttachment(decrypted.group.avatar); } } - if (decrypted.contact && decrypted.contact.length) { - const contacts = decrypted.contact; + decrypted.attachments = (decrypted.attachments || []).map( + this.cleanAttachment.bind(this) + ); + decrypted.preview = (decrypted.preview || []).map(item => { + const { image } = item; - for (let i = 0, max = contacts.length; i < max; i += 1) { - const contact = contacts[i]; - const { avatar } = contact; - - if (avatar && avatar.avatar) { - // We don't want the failure of a thumbnail download to fail the handling of - // this message entirely, like we do for full attachments. - promises.push( - this.handleAttachment(avatar.avatar).catch(error => { - window.log.error( - 'Problem loading avatar for contact', - error && error.stack ? error.stack : error - ); - }) - ); - } + if (!image) { + return item; } - } + + return { + ...item, + image: this.cleanAttachment(image), + }; + }); + decrypted.contact = (decrypted.contact || []).map(item => { + const { avatar } = item; + + if (!avatar || !avatar.avatar) { + return item; + } + + return { + ...item, + avatar: { + ...item.avatar, + avatar: this.cleanAttachment(item.avatar.avatar), + }, + }; + }); if (decrypted.quote && decrypted.quote.id) { decrypted.quote.id = decrypted.quote.id.toNumber(); } - if (decrypted.quote && decrypted.quote.attachments) { - const { attachments } = decrypted.quote; + if (decrypted.quote) { + decrypted.quote.attachments = (decrypted.quote.attachments || []).map( + item => { + const { thumbnail } = item; - for (let i = 0, max = attachments.length; i < max; i += 1) { - const attachment = attachments[i]; - const { thumbnail } = attachment; + if (!thumbnail) { + return item; + } - if (thumbnail) { - // We don't want the failure of a thumbnail download to fail the handling of - // this message entirely, like we do for full attachments. - promises.push( - this.handleAttachment(thumbnail).catch(error => { - window.log.error( - 'Problem loading thumbnail for quote', - error && error.stack ? error.stack : error - ); - }) - ); + return { + ...item, + thumbnail: this.cleanAttachment(item.thumbnail), + }; } - } + ); } return Promise.all(promises).then(() => decrypted); @@ -1454,6 +1454,11 @@ textsecure.MessageReceiver = function MessageReceiverWrapper( ); this.getStatus = messageReceiver.getStatus.bind(messageReceiver); this.close = messageReceiver.close.bind(messageReceiver); + + this.downloadAttachment = messageReceiver.downloadAttachment.bind( + messageReceiver + ); + messageReceiver.connect(); }; diff --git a/stylesheets/_ios.scss b/stylesheets/_ios.scss index 68c1103c5ec8..fae0cada3dc3 100644 --- a/stylesheets/_ios.scss +++ b/stylesheets/_ios.scss @@ -132,6 +132,32 @@ background-color: $color-gray-60; } + .module-spinner__circle--incoming { + background-color: $color-gray-15; + } + .module-spinner__arc--incoming { + background-color: $color-gray-60; + } + .module-spinner__circle--small-incoming { + background-color: $color-gray-15; + } + .module-spinner__arc--small-incoming { + background-color: $color-gray-60; + } + + .module-spinner__circle--outgoing { + background-color: $color-gray-45; + } + .module-spinner__arc--outgoing { + background-color: $color-gray-05; + } + .module-spinner__circle--small-outgoing { + background-color: $color-gray-45; + } + .module-spinner__arc--small-outgoing { + background-color: $color-gray-05; + } + &.dark-theme { // _modules @@ -295,5 +321,31 @@ .module-embedded-contact__contact-method--incoming { color: $color-gray-25; } + + .module-spinner__circle--incoming { + background-color: $color-gray-45; + } + .module-spinner__arc--incoming { + background-color: $color-gray-25; + } + .module-spinner__circle--small-incoming { + background-color: $color-gray-45; + } + .module-spinner__arc--small-incoming { + background-color: $color-gray-25; + } + + .module-spinner__circle--outgoing { + background-color: $color-gray-45; + } + .module-spinner__arc--outgoing { + background-color: $color-gray-05; + } + .module-spinner__circle--small-outgoing { + background-color: $color-gray-45; + } + .module-spinner__arc--small-outgoing { + background-color: $color-gray-05; + } } } diff --git a/stylesheets/_modules.scss b/stylesheets/_modules.scss index 478e20e685c3..856f598a3a35 100644 --- a/stylesheets/_modules.scss +++ b/stylesheets/_modules.scss @@ -204,7 +204,6 @@ // Entirely to ensure that images are centered if they aren't full width of bubble text-align: center; position: relative; - cursor: pointer; margin-left: -12px; margin-right: -12px; @@ -251,6 +250,7 @@ .module-message__generic-attachment { display: flex; flex-direction: row; + align-items: center; } .module-message__generic-attachment--with-content-below { @@ -264,6 +264,10 @@ .module-message__generic-attachment__icon-container { position: relative; } +.module-message__generic-attachment__spinner-container { + padding-left: 4px; + padding-right: 4px; +} .module-message__generic-attachment__icon { background: url('../images/file-gradient.svg') no-repeat center; @@ -967,7 +971,7 @@ cursor: pointer; display: flex; flex-direction: row; - align-items: stretch; + align-items: center; } .module-embedded-contact--with-content-above { @@ -978,6 +982,11 @@ padding-bottom: 4px; } +.module-embedded-contact__spinner-container { + padding-left: 5px; + padding-right: 5px; +} + .module-embedded-contact__text-container { flex-grow: 1; margin-left: 8px; @@ -2157,6 +2166,13 @@ background-color: $color-black-02; } +.module-image__loading-placeholder { + display: inline-flex; + flex-direction: row; + align-items: center; + background-color: $color-black-015; +} + .module-image__image { object-fit: cover; // redundant with attachment-container, but we get cursor flashing on move otherwise @@ -2681,6 +2697,80 @@ @include color-svg('../images/x-16.svg', $color-gray-60); } +// Module: Spinner + +.module-spinner__container { + margin-left: auto; + margin-right: auto; + position: relative; + height: 56px; + width: 56px; +} + +.module-spinner__circle { + position: absolute; + top: 0; + left: 0; + + @include color-svg('../images/spinner-track-56.svg', $color-gray-15); + z-index: 2; + height: 56px; + width: 56px; +} +.module-spinner__arc { + position: absolute; + top: 0; + left: 0; + + @include color-svg('../images/spinner-56.svg', $color-gray-60); + z-index: 3; + height: 56px; + width: 56px; + + animation: spinner-arc-animation 1000ms linear infinite; +} + +@keyframes spinner-arc-animation { + 0% { + transform: rotate(0deg); + } + 50% { + transform: rotate(180deg); + } + 100% { + transform: rotate(360deg); + } +} + +.module-spinner__container--small { + height: 24px; + width: 24px; +} + +.module-spinner__circle--small { + @include color-svg('../images/spinner-track-24.svg', $color-gray-15); + height: 24px; + width: 24px; +} +.module-spinner__arc--small { + @include color-svg('../images/spinner-24.svg', $color-gray-60); + height: 24px; + width: 24px; +} + +.module-spinner__circle--incoming { + background-color: $color-gray-75; +} +.module-spinner__arc--incoming { + background-color: $color-gray-15; +} +.module-spinner__circle--small-incoming { + background-color: $color-gray-75; +} +.module-spinner__arc--small-incoming { + background-color: $color-gray-15; +} + // Third-party module: react-contextmenu .react-contextmenu { diff --git a/stylesheets/_theme_dark.scss b/stylesheets/_theme_dark.scss index 928ef80649b7..204fc4023206 100644 --- a/stylesheets/_theme_dark.scss +++ b/stylesheets/_theme_dark.scss @@ -1326,10 +1326,18 @@ body.dark-theme { // Module: Image + .module-image { + background-color: $color-black; + } + .module-image__border-overlay { box-shadow: inset 0px 0px 0px 1px $color-white-015; } + .module-image__loading-placeholder { + background-color: $color-white-015; + } + // Module: Image Grid // Module: Typing Animation @@ -1401,6 +1409,47 @@ body.dark-theme { @include color-svg('../images/x-16.svg', $color-gray-25); } + // Module: Spinner + + .module-spinner__circle { + background-color: $color-gray-45; + } + .module-spinner__arc { + background-color: $color-gray-05; + } + .module-spinner__circle--small { + background-color: $color-gray-45; + } + .module-spinner__arc--small { + background-color: $color-gray-05; + } + + .module-spinner__circle--incoming { + background-color: $color-gray-45; + } + .module-spinner__arc--incoming { + background-color: $color-gray-05; + } + .module-spinner__circle--small-incoming { + background-color: $color-gray-45; + } + .module-spinner__arc--small-incoming { + background-color: $color-gray-05; + } + + .module-spinner__circle--outgoing { + background-color: $color-gray-45; + } + .module-spinner__arc--outgoing { + background-color: $color-gray-05; + } + .module-spinner__circle--small-outgoing { + background-color: $color-gray-45; + } + .module-spinner__arc--small-outgoing { + background-color: $color-gray-05; + } + // Third-party module: react-contextmenu .react-contextmenu { diff --git a/stylesheets/_variables.scss b/stylesheets/_variables.scss index 320d4af9f577..b91e71e634ca 100644 --- a/stylesheets/_variables.scss +++ b/stylesheets/_variables.scss @@ -109,6 +109,7 @@ $color-dark-60: #797a7c; $color-dark-70: #414347; $color-dark-85: #1a1c20; $color-black-008: rgba($color-black, 0.08); +$color-black-005: rgba($color-black, 0.05); $color-black-008-no-tranparency: #ededed; $color-black-016-no-tranparency: #d9d9d9; $color-black-012: rgba($color-black, 0.12); diff --git a/test/modules/types/message_test.js b/test/modules/types/message_test.js index 09333f377b2c..0231c8b686aa 100644 --- a/test/modules/types/message_test.js +++ b/test/modules/types/message_test.js @@ -575,17 +575,22 @@ describe('Message', () => { body: 'hey there!', quote: { text: 'hey!', - attachments: [], + attachments: [ + { + fileName: 'manifesto.txt', + contentType: 'text/plain', + }, + ], }, }; const result = await upgradeVersion(message, { logger }); assert.deepEqual(result, message); }); - it('eliminates thumbnails with no data field', async () => { + it('does not eliminate thumbnails with missing data field', async () => { const upgradeAttachment = sinon .stub() - .throws(new Error("Shouldn't be called")); + .returns({ fileName: 'processed!' }); const upgradeVersion = Message._mapQuotedAttachments(upgradeAttachment); const message = { @@ -597,7 +602,7 @@ describe('Message', () => { fileName: 'cat.gif', contentType: 'image/gif', thumbnail: { - fileName: 'failed to download!', + fileName: 'not yet downloaded!', }, }, ], @@ -611,6 +616,9 @@ describe('Message', () => { { contentType: 'image/gif', fileName: 'cat.gif', + thumbnail: { + fileName: 'processed!', + }, }, ], }, diff --git a/ts/components/Spinner.md b/ts/components/Spinner.md new file mode 100644 index 000000000000..ee30bbc82caa --- /dev/null +++ b/ts/components/Spinner.md @@ -0,0 +1,15 @@ +#### Large + +```jsx + + + +``` + +#### Small + +```jsx + + + +``` diff --git a/ts/components/Spinner.tsx b/ts/components/Spinner.tsx new file mode 100644 index 000000000000..ad2a326d202a --- /dev/null +++ b/ts/components/Spinner.tsx @@ -0,0 +1,47 @@ +import React from 'react'; +import classNames from 'classnames'; + +interface Props { + small?: boolean; + direction?: string; +} + +export class Spinner extends React.Component { + public render() { + const { small, direction } = this.props; + + return ( +
+
+
+
+ ); + } +} diff --git a/ts/components/conversation/EmbeddedContact.md b/ts/components/conversation/EmbeddedContact.md index 1e0d07471895..1f22d3ab9a46 100644 --- a/ts/components/conversation/EmbeddedContact.md +++ b/ts/components/conversation/EmbeddedContact.md @@ -66,6 +66,51 @@ const contact = { ; ``` +#### Image download pending + +```jsx +const contact = { + name: { + displayName: 'Someone Somewhere', + }, + number: [ + { + value: '(202) 555-0000', + type: 1, + }, + ], + avatar: { + avatar: { + pending: true, + }, + }, + onClick: () => console.log('onClick'), + onSendMessage: () => console.log('onSendMessage'), + hasSignalAccount: true, +}; + +
  • + +
    • +
  • + +
    • +    ; +``` + #### Really long data ``` diff --git a/ts/components/conversation/EmbeddedContact.tsx b/ts/components/conversation/EmbeddedContact.tsx index b9a818d75974..86a32039dfec 100644 --- a/ts/components/conversation/EmbeddedContact.tsx +++ b/ts/components/conversation/EmbeddedContact.tsx @@ -2,6 +2,7 @@ import React from 'react'; import classNames from 'classnames'; import { Avatar } from '../Avatar'; +import { Spinner } from '../Spinner'; import { Contact, getName } from '../../types/Contact'; import { Localizer } from '../../types/Util'; @@ -27,6 +28,7 @@ export class EmbeddedContact extends React.Component { withContentBelow, } = this.props; const module = 'embedded-contact'; + const direction = isIncoming ? 'incoming' : 'outgoing'; return (
    { role="button" onClick={onClick} > - {renderAvatar({ contact, i18n, size: 48 })} + {renderAvatar({ contact, i18n, size: 48, direction })}
    {renderName({ contact, isIncoming, module })} {renderContactShorthand({ contact, isIncoming, module })} @@ -58,16 +60,27 @@ export function renderAvatar({ contact, i18n, size, + direction, }: { contact: Contact; i18n: Localizer; size: number; + direction?: string; }) { const { avatar } = contact; const avatarPath = avatar && avatar.avatar && avatar.avatar.path; + const pending = avatar && avatar.avatar && avatar.avatar.pending; const name = getName(contact) || ''; + if (pending) { + return ( +
    + +
    + ); + } + return ( - - + + + + + + ``` ### Various curved corners ```jsx - - - - + + + + + + + ``` ### With bottom overlay ```jsx - - - + + + + + + ``` ### With play icon ```jsx - - - + + + + + + ``` ### With dark overlay and text ```jsx -
    +
    - - - + + + +
    @@ -46,31 +205,46 @@ height="200" width="199" darkOverlay + attachment={{}} overlayText="+3" url={util.pngObjectUrl} + i18n={util.i18n} /> +
    -
    + ``` ### With caption ```jsx -
    +
    +
    @@ -123,18 +304,28 @@ url={util.pngObjectUrl} i18n={util.i18n} /> +
    -
    + ``` ### With top-right X and soft corners ```jsx -
    +
    console.log('onClick')} onClickClose={attachment => console.log('onClickClose', attachment)} @@ -145,6 +336,7 @@ console.log('onClick')} onClickClose={attachment => console.log('onClickClose', attachment)} @@ -155,6 +347,18 @@ console.log('onClick')} + onClickClose={attachment => console.log('onClickClose', attachment)} + softCorners={true} + url={util.gifObjectUrl} + i18n={util.i18n} + /> + console.log('onClick')} onClickClose={attachment => console.log('onClickClose', attachment)} @@ -168,6 +372,7 @@ console.log('onClick')} @@ -179,6 +384,7 @@ console.log('onClick')} @@ -198,6 +404,17 @@ url={util.gifObjectUrl} i18n={util.i18n} /> + console.log('onClick')} + onClickClose={attachment => console.log('onClickClose', attachment)} + softCorners={true} + url={util.gifObjectUrl} + i18n={util.i18n} + />
    -
    + ``` diff --git a/ts/components/conversation/Image.tsx b/ts/components/conversation/Image.tsx index 9c8079745180..9b932b59d8f2 100644 --- a/ts/components/conversation/Image.tsx +++ b/ts/components/conversation/Image.tsx @@ -1,6 +1,7 @@ import React from 'react'; import classNames from 'classnames'; +import { Spinner } from '../Spinner'; import { Localizer } from '../../types/Util'; import { AttachmentType } from './types'; @@ -59,19 +60,20 @@ export class Image extends React.Component { width, } = this.props; - const { caption } = attachment || { caption: null }; + const { caption, pending } = attachment || { caption: null, pending: true }; + const canClick = onClick && !pending; return (
    { - if (onClick) { + if (canClick) { onClick(attachment); } }} className={classNames( 'module-image', - onClick ? 'module-image__with-click-handler' : null, + canClick ? 'module-image__with-click-handler' : null, curveBottomLeft ? 'module-image--curved-bottom-left' : null, curveBottomRight ? 'module-image--curved-bottom-right' : null, curveTopLeft ? 'module-image--curved-top-left' : null, @@ -80,14 +82,29 @@ export class Image extends React.Component { softCorners ? 'module-image--soft-corners' : null )} > - {alt} + {pending ? ( +
    + +
    + ) : ( + {alt} + )} {caption ? ( { )} /> ) : null} - {playIconOverlay ? ( + {!pending && playIconOverlay ? (
    diff --git a/ts/components/conversation/ImageGrid.tsx b/ts/components/conversation/ImageGrid.tsx index f42424dea282..bb90d04124fb 100644 --- a/ts/components/conversation/ImageGrid.tsx +++ b/ts/components/conversation/ImageGrid.tsx @@ -340,7 +340,11 @@ export function isImageAttachment(attachment: AttachmentType) { ); } export function hasImage(attachments?: Array) { - return attachments && attachments[0] && attachments[0].url; + return ( + attachments && + attachments[0] && + (attachments[0].url || attachments[0].pending) + ); } export function isVideo(attachments?: Array) { diff --git a/ts/components/conversation/Message.md b/ts/components/conversation/Message.md index 79c663f3e09d..98d89acf17e0 100644 --- a/ts/components/conversation/Message.md +++ b/ts/components/conversation/Message.md @@ -1166,6 +1166,111 @@ Note that the delivered indicator is always Signal Blue, not the conversation co ``` +#### Pending images + +``` + +
  • + console.log('onClickAttachment')} + /> +
    • +
  • + console.log('onClickAttachment')} + /> +
    • +
  • + console.log('onClickAttachment')} + /> +
    • +
  • + console.log('onClickAttachment')} + /> +
    • +     +``` + #### Image with portrait aspect ratio ```jsx @@ -2533,6 +2638,84 @@ Voice notes are not shown any differently from audio attachments. ``` +#### Other file type pending + +```jsx + +
  • + console.log('onClickAttachment')} + /> +
    • +
  • + console.log('onClickAttachment')} + /> +
    • +
  • + console.log('onClickAttachment')} + /> +
    • +
  • + console.log('onClickAttachment')} + /> +
    • +     +``` + #### Dangerous file type ```jsx @@ -2799,6 +2982,103 @@ Voice notes are not shown any differently from audio attachments. ``` +#### Link previews with pending image + +```jsx + +
  • + console.log('onClickLinkPreview', url)} + /> +
    • +
  • + console.log('onClickLinkPreview', url)} + /> +
    • +
  • + console.log('onClickLinkPreview', url)} + /> +
    • +
  • + console.log('onClickLinkPreview', url)} + /> +
    • +     +``` + #### Link previews, no image ```jsx diff --git a/ts/components/conversation/Message.tsx b/ts/components/conversation/Message.tsx index c4b2b32d0674..2e8741ef3c75 100644 --- a/ts/components/conversation/Message.tsx +++ b/ts/components/conversation/Message.tsx @@ -2,6 +2,7 @@ import React from 'react'; import classNames from 'classnames'; import { Avatar } from '../Avatar'; +import { Spinner } from '../Spinner'; import { MessageBody } from './MessageBody'; import { ExpireTimer, getIncrement } from './ExpireTimer'; import { @@ -345,7 +346,7 @@ export class Message extends React.Component { />
    ); - } else if (isAudio(attachments)) { + } else if (!firstAttachment.pending && isAudio(attachments)) { return ( ); } else { - const { fileName, fileSize, contentType } = firstAttachment; + const { pending, fileName, fileSize, contentType } = firstAttachment; const extension = getExtension({ contentType, fileName }); const isDangerous = isFileDangerous(fileName || ''); @@ -379,20 +381,26 @@ export class Message extends React.Component { : null )} > -
    -
    - {extension ? ( -
    - {extension} + {pending ? ( +
    + +
    + ) : ( +
    +
    + {extension ? ( +
    + {extension} +
    + ) : null} +
    + {isDangerous ? ( +
    +
    ) : null}
    - {isDangerous ? ( -
    -
    -
    - ) : null} -
    + )}
    { attachments && attachments[0] ? attachments[0].fileName : null; const isDangerous = isFileDangerous(fileName || ''); const multipleAttachments = attachments && attachments.length > 1; + const firstAttachment = attachments && attachments[0]; const downloadButton = - !multipleAttachments && attachments && attachments[0] ? ( + !multipleAttachments && firstAttachment && !firstAttachment.pending ? (
    { if (onDownload) { @@ -983,6 +992,10 @@ export function getExtension({ } } + if (!contentType) { + return null; + } + const slash = contentType.indexOf('/'); if (slash >= 0) { return contentType.slice(slash + 1); diff --git a/ts/components/conversation/Quote.md b/ts/components/conversation/Quote.md index 3668e9ef1bef..ee62ccec0d99 100644 --- a/ts/components/conversation/Quote.md +++ b/ts/components/conversation/Quote.md @@ -1016,6 +1016,50 @@ messages the color is taken from the contact who wrote the quoted message. quote={{ authorColor: 'purple', attachment: { + pending: true, + contentType: 'image/gif', + fileName: 'pi.gif', + }, + authorPhoneNumber: '(202) 555-0011', + }} + /> + + +``` + +#### Pending image download + +```jsx + +
  • + +
    • +
  • + { +export class Quote extends React.Component { + public handleImageErrorBound: () => void; + + public constructor(props: Props) { + super(props); + + this.handleImageErrorBound = this.handleImageError.bind(this); + + this.state = { + imageBroken: false, + }; + } + + public handleImageError() { + // tslint:disable-next-line no-console + console.log('Message: Image failed to load; failing over to placeholder'); + this.setState({ + imageBroken: true, + }); + } + public renderImage(url: string, i18n: Localizer, icon?: string) { const iconElement = icon ? (
    @@ -102,7 +126,11 @@ export class Quote extends React.Component { return (
    - {i18n('quoteThumbnailAlt')} + {i18n('quoteThumbnailAlt')} {iconElement}
    ); @@ -159,6 +187,8 @@ export class Quote extends React.Component { public renderIconContainer() { const { attachment, i18n } = this.props; + const { imageBroken } = this.state; + if (!attachment) { return null; } @@ -167,12 +197,12 @@ export class Quote extends React.Component { const objectUrl = getObjectUrl(thumbnail); if (GoogleChrome.isVideoTypeSupported(contentType)) { - return objectUrl + return objectUrl && !imageBroken ? this.renderImage(objectUrl, i18n, 'play') : this.renderIcon('movie'); } if (GoogleChrome.isImageTypeSupported(contentType)) { - return objectUrl + return objectUrl && !imageBroken ? this.renderImage(objectUrl, i18n) : this.renderIcon('image'); } diff --git a/ts/components/conversation/types.ts b/ts/components/conversation/types.ts index 4789fb8ecd83..79022a73acaf 100644 --- a/ts/components/conversation/types.ts +++ b/ts/components/conversation/types.ts @@ -10,6 +10,7 @@ export interface AttachmentType { url: string; size?: number; fileSize?: string; + pending?: boolean; width?: number; height?: number; screenshot?: { diff --git a/ts/test/types/Contact_test.ts b/ts/test/types/Contact_test.ts index 40a5d63c6d02..c35eeee4d688 100644 --- a/ts/test/types/Contact_test.ts +++ b/ts/test/types/Contact_test.ts @@ -1,6 +1,6 @@ import { assert } from 'chai'; -import { getName } from '../../types/Contact'; +import { contactSelector, getName } from '../../types/Contact'; describe('Contact', () => { describe('getName', () => { @@ -61,4 +61,136 @@ describe('Contact', () => { assert.strictEqual(actual, expected); }); }); + describe('contactSelector', () => { + const regionCode = '1'; + const hasSignalAccount = true; + const getAbsoluteAttachmentPath = (path: string) => `absolute:${path}`; + const onSendMessage = () => null; + const onClick = () => null; + + it('eliminates avatar if it has had an attachment download error', () => { + const contact = { + name: { + displayName: 'displayName', + givenName: 'givenName', + familyName: 'familyName', + }, + organization: 'Somewhere, Inc.', + avatar: { + isProfile: true, + avatar: { + error: true, + }, + }, + }; + const expected = { + name: { + displayName: 'displayName', + givenName: 'givenName', + familyName: 'familyName', + }, + organization: 'Somewhere, Inc.', + avatar: undefined, + hasSignalAccount, + onSendMessage, + onClick, + number: undefined, + }; + const actual = contactSelector(contact, { + regionCode, + hasSignalAccount, + getAbsoluteAttachmentPath, + onSendMessage, + onClick, + }); + assert.deepEqual(actual, expected); + }); + + it('does not calculate absolute path if avatar is pending', () => { + const contact = { + name: { + displayName: 'displayName', + givenName: 'givenName', + familyName: 'familyName', + }, + organization: 'Somewhere, Inc.', + avatar: { + isProfile: true, + avatar: { + pending: true, + }, + }, + }; + const expected = { + name: { + displayName: 'displayName', + givenName: 'givenName', + familyName: 'familyName', + }, + organization: 'Somewhere, Inc.', + avatar: { + isProfile: true, + avatar: { + pending: true, + path: undefined, + }, + }, + hasSignalAccount, + onSendMessage, + onClick, + number: undefined, + }; + const actual = contactSelector(contact, { + regionCode, + hasSignalAccount, + getAbsoluteAttachmentPath, + onSendMessage, + onClick, + }); + assert.deepEqual(actual, expected); + }); + + it('calculates absolute path', () => { + const contact = { + name: { + displayName: 'displayName', + givenName: 'givenName', + familyName: 'familyName', + }, + organization: 'Somewhere, Inc.', + avatar: { + isProfile: true, + avatar: { + path: 'somewhere', + }, + }, + }; + const expected = { + name: { + displayName: 'displayName', + givenName: 'givenName', + familyName: 'familyName', + }, + organization: 'Somewhere, Inc.', + avatar: { + isProfile: true, + avatar: { + path: 'absolute:somewhere', + }, + }, + hasSignalAccount, + onSendMessage, + onClick, + number: undefined, + }; + const actual = contactSelector(contact, { + regionCode, + hasSignalAccount, + getAbsoluteAttachmentPath, + onSendMessage, + onClick, + }); + assert.deepEqual(actual, expected); + }); + }); }); diff --git a/ts/types/Contact.ts b/ts/types/Contact.ts index af63a2282384..c5afbcb41d94 100644 --- a/ts/types/Contact.ts +++ b/ts/types/Contact.ts @@ -63,7 +63,9 @@ interface Avatar { } interface Attachment { - path: string; + path?: string; + error?: boolean; + pending?: boolean; } export function contactSelector( @@ -85,14 +87,20 @@ export function contactSelector( } = options; let { avatar } = contact; - if (avatar && avatar.avatar && avatar.avatar.path) { - avatar = { - ...avatar, - avatar: { - ...avatar.avatar, - path: getAbsoluteAttachmentPath(avatar.avatar.path), - }, - }; + if (avatar && avatar.avatar) { + if (avatar.avatar.error) { + avatar = undefined; + } else { + avatar = { + ...avatar, + avatar: { + ...avatar.avatar, + path: avatar.avatar.path + ? getAbsoluteAttachmentPath(avatar.avatar.path) + : undefined, + }, + }; + } } return {