Page media in Lightbox

2023-03-03 19:03:15 -08:00 · 2023-03-03 19:03:15 -08:00 · 5dff1768bd
commit 5dff1768bd
parent 03697f66e7
16 changed files with 603 additions and 395 deletions
--- a/ts/sql/Client.ts
+++ b/ts/sql/Client.ts
@ -40,6 +40,7 @@ import { cleanupMessage } from '../util/cleanup';
 import { drop } from '../util/drop';

 import type {
+  AdjacentMessagesByConversationOptionsType,
  AllItemsType,
  AttachmentDownloadJobType,
  ClientInterface,
@ -674,77 +675,24 @@ function handleMessageJSON(
 }

 async function getNewerMessagesByConversation(
-  conversationId: string,
-  {
-    includeStoryReplies,
-    limit = 100,
-    receivedAt = 0,
-    sentAt = 0,
-    storyId,
-  }: {
-    includeStoryReplies: boolean;
-    limit?: number;
-    receivedAt?: number;
-    sentAt?: number;
-    storyId: UUIDStringType | undefined;
-  }
+  options: AdjacentMessagesByConversationOptionsType
 ): Promise<Array<MessageType>> {
-  const messages = await channels.getNewerMessagesByConversation(
-    conversationId,
-    {
-      includeStoryReplies,
-      limit,
-      receivedAt,
-      sentAt,
-      storyId,
-    }
-  );
+  const messages = await channels.getNewerMessagesByConversation(options);

  return handleMessageJSON(messages);
 }

 async function getOlderMessagesByConversation(
-  conversationId: string,
-  {
-    includeStoryReplies,
-    limit = 100,
-    messageId,
-    receivedAt = Number.MAX_VALUE,
-    sentAt = Number.MAX_VALUE,
-    storyId,
-  }: {
-    includeStoryReplies: boolean;
-    limit?: number;
-    messageId?: string;
-    receivedAt?: number;
-    sentAt?: number;
-    storyId: string | undefined;
-  }
+  options: AdjacentMessagesByConversationOptionsType
 ): Promise<Array<MessageType>> {
-  const messages = await channels.getOlderMessagesByConversation(
-    conversationId,
-    {
-      includeStoryReplies,
-      limit,
-      receivedAt,
-      sentAt,
-      messageId,
-      storyId,
-    }
-  );
+  const messages = await channels.getOlderMessagesByConversation(options);

  return handleMessageJSON(messages);
 }

-async function getConversationRangeCenteredOnMessage(options: {
-  conversationId: string;
-  includeStoryReplies: boolean;
-  limit?: number;
-  messageId: string;
-  receivedAt: number;
-  sentAt?: number;
-  storyId: UUIDStringType | undefined;
-}): Promise<GetConversationRangeCenteredOnMessageResultType<MessageType>> {
+async function getConversationRangeCenteredOnMessage(
+  options: AdjacentMessagesByConversationOptionsType
+): Promise<GetConversationRangeCenteredOnMessageResultType<MessageType>> {
  const result = await channels.getConversationRangeCenteredOnMessage(options);

  return {
@ -771,7 +719,8 @@ async function removeAllMessagesInConversation(
    // Yes, we really want the await in the loop. We're deleting a chunk at a
    //   time so we don't use too much memory.
    // eslint-disable-next-line no-await-in-loop
-    messages = await getOlderMessagesByConversation(conversationId, {
+    messages = await getOlderMessagesByConversation({
+      conversationId,
      limit: chunkSize,
      includeStoryReplies: true,
      storyId: undefined,
--- a/ts/sql/Interface.ts
+++ b/ts/sql/Interface.ts
@ -19,6 +19,17 @@ import type { RemoveAllConfiguration } from '../types/RemoveAllConfiguration';
 import type { LoggerType } from '../types/Logging';
 import type { ReadStatus } from '../messages/MessageReadStatus';

+export type AdjacentMessagesByConversationOptionsType = Readonly<{
+  conversationId: string;
+  messageId?: string;
+  includeStoryReplies: boolean;
+  limit?: number;
+  receivedAt?: number;
+  sentAt?: number;
+  storyId: string | undefined;
+  requireVisualMediaAttachments?: boolean;
+}>;
+
 export type AttachmentDownloadJobTypeType =
  | 'long-message'
  | 'attachment'
@ -481,7 +492,7 @@ export type DataInterface = {
  getTotalUnreadForConversation: (
    conversationId: string,
    options: {
-      storyId: UUIDStringType | undefined;
+      storyId: string | undefined;
      includeStoryReplies: boolean;
    }
  ) => Promise<number>;
@ -491,12 +502,12 @@ export type DataInterface = {
    newestUnreadAt: number;
    now?: number;
    readAt?: number;
-    storyId?: UUIDStringType;
+    storyId?: string;
  }) => Promise<GetUnreadByConversationAndMarkReadResultType>;
  getUnreadReactionsAndMarkRead: (options: {
    conversationId: string;
    newestUnreadAt: number;
-    storyId?: UUIDStringType;
+    storyId?: string;
  }) => Promise<Array<ReactionResultType>>;
  markReactionAsRead: (
    targetAuthorUuid: string,
@ -536,13 +547,11 @@ export type DataInterface = {
    sourceUuid?: UUIDStringType;
  }) => Promise<GetAllStoriesResultType>;
  // getNewerMessagesByConversation is JSON on server, full message on Client
-  getMessageMetricsForConversation: (
-    conversationId: string,
-    options: {
-      storyId?: UUIDStringType;
-      includeStoryReplies: boolean;
-    }
-  ) => Promise<ConversationMetricsType>;
+  getMessageMetricsForConversation: (options: {
+    conversationId: string;
+    storyId?: string;
+    includeStoryReplies: boolean;
+  }) => Promise<ConversationMetricsType>;
  // getConversationRangeCenteredOnMessage is JSON on server, full message on client
  getConversationMessageStats: (options: {
    conversationId: string;
@ -738,35 +747,14 @@ export type ServerInterface = DataInterface & {
  ) => Promise<Array<ServerSearchResultMessageType>>;

  getOlderMessagesByConversation: (
-    conversationId: string,
-    options: {
-      includeStoryReplies: boolean;
-      limit?: number;
-      messageId?: string;
-      receivedAt?: number;
-      sentAt?: number;
-      storyId: string | undefined;
-    }
+    options: AdjacentMessagesByConversationOptionsType
  ) => Promise<Array<MessageTypeUnhydrated>>;
  getNewerMessagesByConversation: (
-    conversationId: string,
-    options: {
-      includeStoryReplies: boolean;
-      limit?: number;
-      receivedAt?: number;
-      sentAt?: number;
-      storyId: UUIDStringType | undefined;
-    }
+    options: AdjacentMessagesByConversationOptionsType
  ) => Promise<Array<MessageTypeUnhydrated>>;
-  getConversationRangeCenteredOnMessage: (options: {
-    conversationId: string;
-    includeStoryReplies: boolean;
-    limit?: number;
-    messageId: string;
-    receivedAt: number;
-    sentAt?: number;
-    storyId: UUIDStringType | undefined;
-  }) => Promise<
+  getConversationRangeCenteredOnMessage: (
+    options: AdjacentMessagesByConversationOptionsType
+  ) => Promise<
    GetConversationRangeCenteredOnMessageResultType<MessageTypeUnhydrated>
  >;

@ -843,35 +831,14 @@ export type ClientExclusiveInterface = {
  ) => Promise<Array<ClientSearchResultMessageType>>;

  getOlderMessagesByConversation: (
-    conversationId: string,
-    options: {
-      includeStoryReplies: boolean;
-      limit?: number;
-      messageId?: string;
-      receivedAt?: number;
-      sentAt?: number;
-      storyId: string | undefined;
-    }
+    options: AdjacentMessagesByConversationOptionsType
  ) => Promise<Array<MessageAttributesType>>;
  getNewerMessagesByConversation: (
-    conversationId: string,
-    options: {
-      includeStoryReplies: boolean;
-      limit?: number;
-      receivedAt?: number;
-      sentAt?: number;
-      storyId: UUIDStringType | undefined;
-    }
+    options: AdjacentMessagesByConversationOptionsType
  ) => Promise<Array<MessageAttributesType>>;
-  getConversationRangeCenteredOnMessage: (options: {
-    conversationId: string;
-    includeStoryReplies: boolean;
-    limit?: number;
-    messageId: string;
-    receivedAt: number;
-    sentAt?: number;
-    storyId: UUIDStringType | undefined;
-  }) => Promise<GetConversationRangeCenteredOnMessageResultType<MessageType>>;
+  getConversationRangeCenteredOnMessage: (
+    options: AdjacentMessagesByConversationOptionsType
+  ) => Promise<GetConversationRangeCenteredOnMessageResultType<MessageType>>;

  createOrUpdateIdentityKey: (data: IdentityKeyType) => Promise<void>;
  getIdentityKeyById: (
--- a/ts/sql/Server.ts
+++ b/ts/sql/Server.ts
@ -71,6 +71,7 @@ import {
 import { updateSchema } from './migrations';

 import type {
+  AdjacentMessagesByConversationOptionsType,
  StoredAllItemsType,
  AttachmentDownloadJobType,
  ConversationMetricsType,
@ -2212,7 +2213,7 @@ async function getUnreadByConversationAndMarkRead({
  conversationId: string;
  includeStoryReplies: boolean;
  newestUnreadAt: number;
-  storyId?: UUIDStringType;
+  storyId?: string;
  readAt?: number;
  now?: number;
 }): Promise<GetUnreadByConversationAndMarkReadResultType> {
@ -2315,7 +2316,7 @@ async function getUnreadReactionsAndMarkRead({
 }: {
  conversationId: string;
  newestUnreadAt: number;
-  storyId?: UUIDStringType;
+  storyId?: string;
 }): Promise<Array<ReactionResultType>> {
  const db = getInstance();

@ -2477,64 +2478,106 @@ async function _removeAllReactions(): Promise<void> {
  db.prepare<EmptyQuery>('DELETE from reactions;').run();
 }

-async function getOlderMessagesByConversation(
-  conversationId: string,
-  options: {
-    includeStoryReplies: boolean;
-    limit?: number;
-    messageId?: string;
-    receivedAt?: number;
-    sentAt?: number;
-    storyId: string | undefined;
-  }
-): Promise<Array<MessageTypeUnhydrated>> {
-  return getOlderMessagesByConversationSync(conversationId, options);
+enum AdjacentDirection {
+  Older = 'Older',
+  Newer = 'Newer',
 }
-function getOlderMessagesByConversationSync(
-  conversationId: string,
+
+function getAdjacentMessagesByConversationSync(
+  direction: AdjacentDirection,
  {
+    conversationId,
    includeStoryReplies,
    limit = 100,
    messageId,
-    receivedAt = Number.MAX_VALUE,
-    sentAt = Number.MAX_VALUE,
+    receivedAt = direction === AdjacentDirection.Older ? Number.MAX_VALUE : 0,
+    sentAt = direction === AdjacentDirection.Older ? Number.MAX_VALUE : 0,
+    requireVisualMediaAttachments,
    storyId,
-  }: {
-    includeStoryReplies: boolean;
-    limit?: number;
-    messageId?: string;
-    receivedAt?: number;
-    sentAt?: number;
-    storyId: string | undefined;
-  }
+  }: AdjacentMessagesByConversationOptionsType
 ): Array<MessageTypeUnhydrated> {
  const db = getInstance();

-  return db
-    .prepare<Query>(
-      `
-      SELECT json FROM messages WHERE
-        conversationId = $conversationId AND
-        ($messageId IS NULL OR id IS NOT $messageId) AND
-        isStory IS 0 AND
-        (${_storyIdPredicate(storyId, includeStoryReplies)}) AND
+  const timeFilter =
+    direction === AdjacentDirection.Older
+      ? `
+      (received_at = $received_at AND sent_at < $sent_at) OR
+      received_at < $received_at
+    `
+      : `
+      (received_at = $received_at AND sent_at > $sent_at) OR
+      received_at > $received_at
+    `;
+
+  const timeOrder = direction === AdjacentDirection.Older ? 'DESC' : 'ASC';
+
+  const requireDifferentMessage =
+    direction === AdjacentDirection.Older || requireVisualMediaAttachments;
+
+  let query = `
+    SELECT json FROM messages WHERE
+      conversationId = $conversationId AND
+      ${
+        requireDifferentMessage
+          ? '($messageId IS NULL OR id IS NOT $messageId) AND'
+          : ''
+      }
+      ${
+        requireVisualMediaAttachments
+          ? 'hasVisualMediaAttachments IS 1 AND'
+          : ''
+      }
+      isStory IS 0 AND
+      (${_storyIdPredicate(storyId, includeStoryReplies)}) AND
+      (
+        ${timeFilter}
+      )
+    ORDER BY received_at ${timeOrder}, sent_at ${timeOrder}
+  `;
+
+  // See `filterValidAttachments` in ts/state/ducks/lightbox.ts
+  if (requireVisualMediaAttachments) {
+    query = `
+      SELECT json
+      FROM (${query}) as messages
+      WHERE
        (
-          (received_at = $received_at AND sent_at < $sent_at) OR
-          received_at < $received_at
-        )
-      ORDER BY received_at DESC, sent_at DESC
+          SELECT COUNT(*)
+          FROM json_each(messages.json ->> 'attachments') AS attachment
+          WHERE
+            attachment.value ->> 'thumbnail' IS NOT NULL AND
+            attachment.value ->> 'pending' IS NOT 1 AND
+            attachment.value ->> 'error' IS NULL
+        ) > 0
      LIMIT $limit;
-      `
-    )
-    .all({
-      conversationId,
-      limit,
-      messageId: messageId || null,
-      received_at: receivedAt,
-      sent_at: sentAt,
-      storyId: storyId || null,
-    })
-    .reverse();
+    `;
+  } else {
+    query = `${query} LIMIT $limit`;
+  }
+
+  const results = db.prepare<Query>(query).all({
+    conversationId,
+    limit,
+    messageId: messageId || null,
+    received_at: receivedAt,
+    sent_at: sentAt,
+    storyId: storyId || null,
+  });
+
+  if (direction === AdjacentDirection.Older) {
+    results.reverse();
+  }
+
+  return results;
+}
+
+async function getOlderMessagesByConversation(
+  options: AdjacentMessagesByConversationOptionsType
+): Promise<Array<MessageTypeUnhydrated>> {
+  return getAdjacentMessagesByConversationSync(
+    AdjacentDirection.Older,
+    options
+  );
 }

 async function getAllStories({
@ -2587,58 +2630,12 @@ async function getAllStories({
 }

 async function getNewerMessagesByConversation(
-  conversationId: string,
-  options: {
-    includeStoryReplies: boolean;
-    limit?: number;
-    receivedAt?: number;
-    sentAt?: number;
-    storyId: UUIDStringType | undefined;
-  }
+  options: AdjacentMessagesByConversationOptionsType
 ): Promise<Array<MessageTypeUnhydrated>> {
-  return getNewerMessagesByConversationSync(conversationId, options);
-}
-function getNewerMessagesByConversationSync(
-  conversationId: string,
-  {
-    includeStoryReplies,
-    limit = 100,
-    receivedAt = 0,
-    sentAt = 0,
-    storyId,
-  }: {
-    includeStoryReplies: boolean;
-    limit?: number;
-    receivedAt?: number;
-    sentAt?: number;
-    storyId: UUIDStringType | undefined;
-  }
-): Array<MessageTypeUnhydrated> {
-  const db = getInstance();
-  const rows: JSONRows = db
-    .prepare<Query>(
-      `
-      SELECT json FROM messages WHERE
-        conversationId = $conversationId AND
-        isStory IS 0 AND
-        (${_storyIdPredicate(storyId, includeStoryReplies)}) AND
-        (
-          (received_at = $received_at AND sent_at > $sent_at) OR
-          received_at > $received_at
-        )
-      ORDER BY received_at ASC, sent_at ASC
-      LIMIT $limit;
-      `
-    )
-    .all({
-      conversationId,
-      limit,
-      received_at: receivedAt,
-      sent_at: sentAt,
-      storyId: storyId || null,
-    });
-
-  return rows;
+  return getAdjacentMessagesByConversationSync(
+    AdjacentDirection.Newer,
+    options
+  );
 }
 function getOldestMessageForConversation(
  conversationId: string,
@ -2646,7 +2643,7 @@ function getOldestMessageForConversation(
    storyId,
    includeStoryReplies,
  }: {
-    storyId?: UUIDStringType;
+    storyId?: string;
    includeStoryReplies: boolean;
  }
 ): MessageMetricsType | undefined {
@ -2679,7 +2676,7 @@ function getNewestMessageForConversation(
    storyId,
    includeStoryReplies,
  }: {
-    storyId?: UUIDStringType;
+    storyId?: string;
    includeStoryReplies: boolean;
  }
 ): MessageMetricsType | undefined {
@ -2842,7 +2839,7 @@ function getOldestUnseenMessageForConversation(
    storyId,
    includeStoryReplies,
  }: {
-    storyId?: UUIDStringType;
+    storyId?: string;
    includeStoryReplies: boolean;
  }
 ): MessageMetricsType | undefined {
@ -2874,7 +2871,7 @@ function getOldestUnseenMessageForConversation(
 async function getTotalUnreadForConversation(
  conversationId: string,
  options: {
-    storyId: UUIDStringType | undefined;
+    storyId: string | undefined;
    includeStoryReplies: boolean;
  }
 ): Promise<number> {
@ -2886,7 +2883,7 @@ function getTotalUnreadForConversationSync(
    storyId,
    includeStoryReplies,
  }: {
-    storyId: UUIDStringType | undefined;
+    storyId: string | undefined;
    includeStoryReplies: boolean;
  }
 ): number {
@ -2917,7 +2914,7 @@ function getTotalUnseenForConversationSync(
    storyId,
    includeStoryReplies,
  }: {
-    storyId?: UUIDStringType;
+    storyId?: string;
    includeStoryReplies: boolean;
  }
 ): number {
@ -2943,22 +2940,19 @@ function getTotalUnseenForConversationSync(
  return row;
 }

-async function getMessageMetricsForConversation(
-  conversationId: string,
-  options: {
-    storyId?: UUIDStringType;
-    includeStoryReplies: boolean;
-  }
-): Promise<ConversationMetricsType> {
-  return getMessageMetricsForConversationSync(conversationId, options);
+async function getMessageMetricsForConversation(options: {
+  conversationId: string;
+  storyId?: string;
+  includeStoryReplies: boolean;
+}): Promise<ConversationMetricsType> {
+  return getMessageMetricsForConversationSync(options);
 }
-function getMessageMetricsForConversationSync(
-  conversationId: string,
-  options: {
-    storyId?: UUIDStringType;
-    includeStoryReplies: boolean;
-  }
-): ConversationMetricsType {
+function getMessageMetricsForConversationSync(options: {
+  conversationId: string;
+  storyId?: string;
+  includeStoryReplies: boolean;
+}): ConversationMetricsType {
+  const { conversationId } = options;
  const oldest = getOldestMessageForConversation(conversationId, options);
  const newest = getNewestMessageForConversation(conversationId, options);
  const oldestUnseen = getOldestUnseenMessageForConversation(
@ -2980,48 +2974,24 @@ function getMessageMetricsForConversationSync(
  };
 }

-async function getConversationRangeCenteredOnMessage({
-  conversationId,
-  includeStoryReplies,
-  limit,
-  messageId,
-  receivedAt,
-  sentAt,
-  storyId,
-}: {
-  conversationId: string;
-  includeStoryReplies: boolean;
-  limit?: number;
-  messageId: string;
-  receivedAt: number;
-  sentAt?: number;
-  storyId: UUIDStringType | undefined;
-}): Promise<
+async function getConversationRangeCenteredOnMessage(
+  options: AdjacentMessagesByConversationOptionsType
+): Promise<
  GetConversationRangeCenteredOnMessageResultType<MessageTypeUnhydrated>
 > {
  const db = getInstance();

  return db.transaction(() => {
    return {
-      older: getOlderMessagesByConversationSync(conversationId, {
-        includeStoryReplies,
-        limit,
-        messageId,
-        receivedAt,
-        sentAt,
-        storyId,
-      }),
-      newer: getNewerMessagesByConversationSync(conversationId, {
-        includeStoryReplies,
-        limit,
-        receivedAt,
-        sentAt,
-        storyId,
-      }),
-      metrics: getMessageMetricsForConversationSync(conversationId, {
-        storyId,
-        includeStoryReplies,
-      }),
+      older: getAdjacentMessagesByConversationSync(
+        AdjacentDirection.Older,
+        options
+      ),
+      newer: getAdjacentMessagesByConversationSync(
+        AdjacentDirection.Newer,
+        options
+      ),
+      metrics: getMessageMetricsForConversationSync(options),
    };
  })();
 }
@ -4998,11 +4968,15 @@ async function getMessagesWithVisualMediaAttachments(
  const rows: JSONRows = db
    .prepare<Query>(
      `
-      SELECT json FROM messages WHERE
+      SELECT json FROM messages
+      INDEXED BY messages_hasVisualMediaAttachments
+      WHERE
        isStory IS 0 AND
        storyId IS NULL AND
        conversationId = $conversationId AND
-        hasVisualMediaAttachments = 1
+        -- Note that this check has to use 'IS' to utilize
+        -- 'messages_hasVisualMediaAttachments' INDEX
+        hasVisualMediaAttachments IS 1
      ORDER BY received_at DESC, sent_at DESC
      LIMIT $limit;
      `
--- a/ts/sql/migrations/79-paging-lightbox.ts
+++ b/ts/sql/migrations/79-paging-lightbox.ts
@ -0,0 +1,32 @@
+// Copyright 2023 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import type { Database } from '@signalapp/better-sqlite3';
+
+import type { LoggerType } from '../../types/Logging';
+
+export default function updateToSchemaVersion79(
+  currentVersion: number,
+  db: Database,
+  logger: LoggerType
+): void {
+  if (currentVersion >= 79) {
+    return;
+  }
+
+  db.transaction(() => {
+    db.exec(`
+      DROP INDEX   messages_hasVisualMediaAttachments;
+      CREATE INDEX messages_hasVisualMediaAttachments
+        ON messages (
+          conversationId, isStory, storyId,
+          hasVisualMediaAttachments, received_at, sent_at
+        )
+        WHERE hasVisualMediaAttachments IS 1;
+    `);
+
+    db.pragma('user_version = 79');
+  })();
+
+  logger.info('updateToSchemaVersion79: success!');
+}
--- a/ts/sql/migrations/index.ts
+++ b/ts/sql/migrations/index.ts
@ -54,6 +54,7 @@ import updateToSchemaVersion75 from './75-noop';
 import updateToSchemaVersion76 from './76-optimize-convo-open-2';
 import updateToSchemaVersion77 from './77-signal-tokenizer';
 import updateToSchemaVersion78 from './78-merge-receipt-jobs';
+import updateToSchemaVersion79 from './79-paging-lightbox';

 function updateToSchemaVersion1(
  currentVersion: number,
@ -1977,6 +1978,7 @@ export const SCHEMA_VERSIONS = [
  updateToSchemaVersion76,
  updateToSchemaVersion77,
  updateToSchemaVersion78,
+  updateToSchemaVersion79,
 ];

 export function updateSchema(db: Database, logger: LoggerType): void {