[keyserver] Update `sendMultimediaMsgReqInpValidator` to accept list of `mediaMessageContents` in addition to `mediaIDs`
ClosedPublic
Actions

Authored by atul on Sep 5 2022, 7:49 PM.

Details

Reviewers

• abosh
tomek
marcin

Commits

rCOMM50bd34ebe9dd: [keyserver] Update `sendMultimediaMsgReqInpValidator` to accept list of…

Summary

In the past, the input to multimediaMessageCreationResponder included the threadID, localID, and mediaIDs.

mediaIDs contained the list of upload IDs for the multimedia message being created. This worked fine under the assumption that there was a 1:1 relationship between media and uploads. However, we break that assumption with video messages...

Going forward we will need to support a 1:many relationship between media and uploads. Specifically with video messages we have 2 uploads (actual video and thumbnail image) for a given video.

The problem is that we need to communicate this more complex relationship between media and uploads to the keyserver when creating a media message. The way we're going to do this is by modifying multimediaMessageCreationResponder to also accept a mediaMessageContents (of type MediaMessageServerDBContent) parameter that encodes the relationship between messages and uploads.

This diff just modifies the validator to support both types of inputs.

Depends on D4990

Test Plan

Able to send image/video messages as expected with the old parameters and "legacy" responder. AKA nothing breaks for existing flows.

Diff Detail

Repository

rCOMM Comm

Branch

arcpatch-D5058 (branched from master)

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

atul created this revision.Sep 5 2022, 7:49 PM

Herald added a subscriber: ashoat. · View Herald TranscriptSep 5 2022, 7:49 PM

atul edited the summary of this revision. (Show Details)Sep 5 2022, 7:49 PM

Harbormaster completed remote builds in B11896: Diff 16339.Sep 5 2022, 7:58 PM

atul requested review of this revision.Sep 5 2022, 7:58 PM

atul mentioned this in D5059: [lib] Rename `sendMultimediaMessage` => `legacySendMultimediaMessage` in `message-actions.js`.Sep 5 2022, 8:44 PM

atul added a child revision: D5059: [lib] Rename `sendMultimediaMessage` => `legacySendMultimediaMessage` in `message-actions.js`.Sep 5 2022, 8:46 PM

atul removed a reviewer: • jacek.Sep 7 2022, 10:14 PM

atul added 1 blocking reviewer(s): tomek.Sep 7 2022, 10:21 PM

tomek accepted this revision.Sep 8 2022, 7:57 AM

tomek added inline comments.

keyserver/src/responders/message-responders.js
109–113 ↗	(On Diff #16339)	What do you think about validating against different shape based on client code version? I think we can do that easily because we have access to `viewer` object here.

This revision is now accepted and ready to land.Sep 8 2022, 7:57 AM

atul added inline comments.Sep 8 2022, 9:08 AM

keyserver/src/responders/message-responders.js
109–113 ↗	(On Diff #16339)	I think that's a good idea. I have it jotted down as one of the things to do as part of "flipping the switch." I think it's okay to defer it for now until we know the minimum codeVersion that will have video messages enabled. Not including that check allows any client to send either the new or old payload... which shouldn't cause any issues and is a little easier to work with while developing.