Page MenuHomePhabricator

[keyserver] Add thumbhash to upload endpoints
ClosedPublic

Authored by bartek on May 15 2023, 1:04 AM.
Tags
None
Referenced Files
F3387456: D7803.diff
Fri, Nov 29, 9:35 AM
F3386348: D7803.id.diff
Fri, Nov 29, 4:16 AM
Unknown Object (File)
Sun, Nov 10, 8:03 PM
Unknown Object (File)
Mon, Nov 4, 1:16 PM
Unknown Object (File)
Fri, Nov 1, 12:23 PM
Unknown Object (File)
Oct 30 2024, 5:57 PM
Unknown Object (File)
Oct 30 2024, 5:57 PM
Unknown Object (File)
Oct 30 2024, 5:57 PM
Subscribers

Details

Summary

This adds possibility to store thumbhash on the keyserver. The thumbhash is saved to the extra column of the uploads table. Despite a deditated column being discussed, I decided to use the extras one for the sake of simplicity (avoid migration), and also because there are few other fields to consider when redesigning the uploads table schema (e.g. blob holder or encryption key which are even more important than the thumbhash).

Depends on D7802

Test Plan

Added the thumbhash property to the upload multimedia endpoint call in input-state-container. Checked with TablePlus that it's actually saved to the database.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bartek held this revision as a draft.
bartek published this revision for review.May 15 2023, 6:33 AM
ashoat requested changes to this revision.May 15 2023, 12:34 PM

Only requesting changes because I think you accidentally left a sleep call in there

keyserver/src/uploads/media-utils.js
71 ↗(On Diff #26438)

Should eg. inputLoop and inputDimensions be in passthroughParams as well?

keyserver/src/uploads/uploads.js
203 ↗(On Diff #26438)

I think you accidentally left this sleep(3000) in?

lib/actions/upload-actions.js
17–22 ↗(On Diff #26438)
  1. Since it's already wrapped in Shape, the ? should not be necessary (but let me know if I'm missing something)
  2. Should we make this read-only?
This revision now requires changes to proceed.May 15 2023, 12:34 PM
keyserver/src/uploads/media-utils.js
71 ↗(On Diff #26438)

inputLoop can be added, for inputDimensions Flow complains (multiple tuple types)

keyserver/src/uploads/uploads.js
203 ↗(On Diff #26438)

My bad

lib/actions/upload-actions.js
17–22 ↗(On Diff #26438)
  1. Yes, you're right
  1. I think Shape already makes object read-only, that's why it wasn't read-only at first place
export type Shape<O> = $ReadOnly<$Rest<O, { ... }>>;

Removed accidental sleep. Updated types as suggested

ashoat added inline comments.
lib/actions/upload-actions.js
21 ↗(On Diff #26503)

Not sure the ? is necessary here – I thought Shape handles this, but I could be wrong

This revision is now accepted and ready to land.May 16 2023, 6:00 AM