Page MenuHomePhabricator

[keyserver] Introduce updated `multimediaMessageCreationResponder(...)` to handle updated payload
ClosedPublic

Authored by atul on Sep 13 2022, 4:25 PM.
Tags
None
Referenced Files
F3347205: D5127.id16641.diff
Fri, Nov 22, 11:19 AM
F3345423: D5127.diff
Fri, Nov 22, 6:00 AM
Unknown Object (File)
Mon, Nov 11, 2:51 AM
Unknown Object (File)
Thu, Nov 7, 1:46 AM
Unknown Object (File)
Thu, Nov 7, 12:22 AM
Unknown Object (File)
Sat, Nov 2, 1:14 PM
Unknown Object (File)
Sat, Nov 2, 1:14 PM
Unknown Object (File)
Sat, Nov 2, 1:14 PM

Details

Summary

We updated the create_multimedia_message endpoint to be able to handle payloads of type SendMultimediaMessageRequest:

export type SendMultimediaMessageRequest =
  | {
      +threadID: string,
      +localID: string,
      +mediaIDs: $ReadOnlyArray<string>,
    }
  | {
      +threadID: string,
      +localID: string,
      +mediaMessageContents: $ReadOnlyArray<MediaMessageServerDBContent>,
    };

The payload which contains mediaMessageContents is the "new" payload which enables us to create video messages. The MediaMessageServerDBContent object encodes the relationship between a media (photo or video) and corresponding upload(s). By passing that object to the keyserver, we're able to properly "reconstruct the media" in multimediaMessageCreationResponder(...).

This diff fully implements multimediaMessageCreationResponder(...) which is heavily influenced by the existing legacyMultimediaMessageCreationResponder(...).

The two main differences are:

  1. We use the newly created fetchMediaFromMediaMessageContent(viewer, mediaMessageContents) instead of the legacy fetchMedia(viewer, mediaIDs) in order to be able to properly reconstruct media of type Video.
  2. We use the newly created assignMessageContainerToMedia(viewer, mediaMessageContents, id) to assign the correct container to all uploads associated with the mediaMessage (including thumbnails).

Depends on D5087

Test Plan

Set breakpoints throughout multimediaMessageCreationResponder(...) and ensured that values were as expected at each point.
Checked uploads table of the serverDB to ensure that rows appeared as expected for each media message.

Diff Detail

Repository
rCOMM Comm
Branch
master
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

keyserver/src/responders/message-responders.js
154

createMessages(...) includes the following:

const content = messageSpecs[messageData.type].messageContentForServerDB?.(
      messageData,
    );

which is where the content column of the message is determined.

I haven't modified multimedia-message-spec.js yet, so it'll still be an array of IDs (strings) for now.

atul requested review of this revision.Sep 13 2022, 4:35 PM
tomek requested changes to this revision.Sep 15 2022, 9:54 AM

I think we should extract common logic / have a function that handles both requests / something else. The reason is that on the keyserver side we need to support both versions for an extended period (until we deprecate 14X clients, so probably a couple of years).

This revision now requires changes to proceed.Sep 15 2022, 9:54 AM

That's fair. It was easier to develop with them split apart... but now that the "new" version of multimediaMessageCreationResponder has been "figured out," it can be consolidated with legacyMultimediaMessageCreationResponder fairly easily.

I'll consolidate the two and then update this diff

atul requested review of this revision.Sep 20 2022, 4:23 AM
In D5127#150659, @tomek wrote:

I think we should extract common logic / have a function that handles both requests / something else. The reason is that on the keyserver side we need to support both versions for an extended period (until we deprecate 14X clients, so probably a couple of years).

Addressed this feedback here: D5191

This revision is now accepted and ready to land.Sep 21 2022, 6:22 AM