From d4b74db05c1f13e7624cc83d34c8dd3c51a4fcee Mon Sep 17 00:00:00 2001 From: Alvaro <110414366+alvaro-signal@users.noreply.github.com> Date: Wed, 10 Aug 2022 11:48:33 -0600 Subject: [PATCH] Don't create preview icon for links with no image (quotes) --- ts/models/conversations.ts | 18 ++----- ts/test-both/util/iterables_test.ts | 47 ++++++++++++++++++ ts/test-electron/models/conversations_test.ts | 48 +++++++++++++++++++ ts/util/iterables.ts | 44 +++++++++++++++++ 4 files changed, 143 insertions(+), 14 deletions(-) diff --git a/ts/models/conversations.ts b/ts/models/conversations.ts index 40d3f1e91..a64409726 100644 --- a/ts/models/conversations.ts +++ b/ts/models/conversations.ts @@ -80,6 +80,7 @@ import { take, repeat, zipObject, + collect, } from '../util/iterables'; import * as universalExpireTimer from '../util/universalExpireTimer'; import type { GroupNameCollisionsWithIdsByTitle } from '../util/groupMemberNameCollisions'; @@ -3640,22 +3641,11 @@ export class ConversationModel extends window.Backbone } if (preview && preview.length) { - const previewsToUse = take(preview, 1); + const previewImages = collect(preview, prev => prev.image); + const previewImagesToUse = take(previewImages, 1); return Promise.all( - map(previewsToUse, async attachment => { - const { image } = attachment; - - if (!image) { - return { - contentType: IMAGE_JPEG, - // Our protos library complains about these fields being undefined, so we - // force them to null - fileName: null, - thumbnail: null, - }; - } - + map(previewImagesToUse, async image => { const { contentType } = image; return { diff --git a/ts/test-both/util/iterables_test.ts b/ts/test-both/util/iterables_test.ts index 709bbe125..ce9ae9660 100644 --- a/ts/test-both/util/iterables_test.ts +++ b/ts/test-both/util/iterables_test.ts @@ -5,6 +5,7 @@ import { assert } from 'chai'; import * as sinon from 'sinon'; import { + collect, concat, every, filter, @@ -251,6 +252,52 @@ describe('iterable utilities', () => { }); }); + describe('collect', () => { + it('returns an empty iterable when passed an empty iterable', () => { + const fn = sinon.fake(); + + assert.deepEqual([...collect([], fn)], []); + assert.deepEqual([...collect(new Set(), fn)], []); + assert.deepEqual([...collect(new Map(), fn)], []); + + sinon.assert.notCalled(fn); + }); + + it('returns a new iterator with some values removed', () => { + const getB = sinon.fake((v: { a: string; b?: number }) => v.b); + const result = collect( + [{ a: 'n' }, { a: 'm', b: 0 }, { a: 'o' }, { a: 'p', b: 1 }], + getB + ); + + sinon.assert.notCalled(getB); + + assert.deepEqual([...result], [0, 1]); + assert.notInstanceOf(result, Array); + + sinon.assert.callCount(getB, 4); + }); + + it('can collect an infinite iterable', () => { + const everyNumber = { + *[Symbol.iterator]() { + for (let i = 0; true; i += 1) { + yield { a: 'x', ...(i % 2 ? { b: i } : {}) }; + } + }, + }; + + const getB = sinon.fake((v: { a: string; b?: number }) => v.b); + const result = collect(everyNumber, getB); + const iterator = result[Symbol.iterator](); + + assert.deepEqual(iterator.next(), { value: 1, done: false }); + assert.deepEqual(iterator.next(), { value: 3, done: false }); + assert.deepEqual(iterator.next(), { value: 5, done: false }); + assert.deepEqual(iterator.next(), { value: 7, done: false }); + }); + }); + describe('find', () => { const isOdd = (n: number) => Boolean(n % 2); diff --git a/ts/test-electron/models/conversations_test.ts b/ts/test-electron/models/conversations_test.ts index 824efacc9..da93c9d81 100644 --- a/ts/test-electron/models/conversations_test.ts +++ b/ts/test-electron/models/conversations_test.ts @@ -3,6 +3,7 @@ import { assert } from 'chai'; import { SendStatus } from '../../messages/MessageSendState'; +import { IMAGE_PNG } from '../../types/MIME'; import { UUID } from '../../types/UUID'; describe('Conversations', () => { @@ -104,4 +105,51 @@ describe('Conversations', () => { assert.strictEqual(conversation.get('lastMessage'), ''); }); + + it('only produces attachments on a quote with an image', async () => { + // Creating a fake conversation + const conversation = new window.Whisper.Conversation({ + avatars: [], + id: UUID.generate().toString(), + e164: '+15551234567', + uuid: UUID.generate().toString(), + type: 'private', + inbox_position: 0, + isPinned: false, + markedUnread: false, + lastMessageDeletedForEveryone: false, + messageCount: 0, + sentMessageCount: 0, + profileSharing: true, + version: 0, + }); + + const resultNoImage = await conversation.getQuoteAttachment( + [], + [ + { + url: 'https://sometest.signal.org/', + }, + ] + ); + + assert.deepEqual(resultNoImage, []); + + const [resultWithImage] = await conversation.getQuoteAttachment( + [], + [ + { + url: 'https://sometest.signal.org/', + image: { + contentType: IMAGE_PNG, + size: 100, + data: new Uint8Array(), + }, + }, + ] + ); + + assert.equal(resultWithImage.contentType, 'image/png'); + assert.equal(resultWithImage.fileName, null); + }); }); diff --git a/ts/util/iterables.ts b/ts/util/iterables.ts index f02240f81..92d51753d 100644 --- a/ts/util/iterables.ts +++ b/ts/util/iterables.ts @@ -101,6 +101,50 @@ class FilterIterator implements Iterator { } } +/** + * Filter and transform (map) that produces a new type + * useful when traversing through fields that might be undefined + */ +export function collect( + iterable: Iterable, + fn: (value: T) => S | undefined +): Iterable { + return new CollectIterable(iterable, fn); +} + +class CollectIterable implements Iterable { + constructor( + private readonly iterable: Iterable, + private readonly fn: (value: T) => S | undefined + ) {} + + [Symbol.iterator](): Iterator { + return new CollectIterator(this.iterable[Symbol.iterator](), this.fn); + } +} + +class CollectIterator implements Iterator { + constructor( + private readonly iterator: Iterator, + private readonly fn: (value: T) => S | undefined + ) {} + + next(): IteratorResult { + // eslint-disable-next-line no-constant-condition + while (true) { + const nextIteration = this.iterator.next(); + if (nextIteration.done) return nextIteration; + const nextValue = this.fn(nextIteration.value); + if (nextValue !== undefined) { + return { + done: false, + value: nextValue, + }; + } + } + } +} + export function find( iterable: Iterable, predicate: (value: T) => unknown