fix(server): use correct file extension for motion photo videos

[lukashass]

Closes #8658

Set originalFileName extension of motion assets to mp4

TODO

• migration for existing assets

[lukashass]

@alextran1502 I saw that since #7679 the file extension for the archive download is only taken from originalFileName. Maybe a better fix than this one would be to set the correct extension in that property in the first place 🤔

[danieldietzler]

Yeah, IMO originalFileName should always have the correct file name.

[lukashass]

On the other hand I'm wondering if the video should be downloaded at all, since it seems to still be embedded in the downloaded jpg and thus downloaded twice.

[jrasm91]

I think we should probably fix the original filename which is wrong, no?

[lukashass]

The video is extracted from the jpg on upload, so the filename is kind of correct (since it says "original").

It is a little ambiguous, that's why I'm questioning the download of the separate video altogether 🤔

[lukashass]

So from what I'm reading there are motion photo formats which

• embed the video in the jpg
• are two separate files from the beginning

This means the correct fix for the issue is to only download a separate video when the jpg does not contain the video, right?

How would we detect that? I could think of two ways:

• save this information (separate/embedded video) in a new property
• compare the file extension of originalFileName of video and photo, if the same, do not download the video

[jrasm91]

Looks great imo. Once the tests are passing I think we'll be good to go.

[lukashass]

@jrasm91 I added the originalFileName to the asset stub that is used in the metadata test.

This tripped up another a "storage template move" test because moving uses asset ids (const filename = asset.originalFileName || asset.id;) as a fallback for non-existent originalFileName.

Outside of tests, I don't see how an asset can not have an originalFileName. This feels a little like a bug, but maybe I'm missing something.

Should I just create a new stub to not interfere with other tests?

[jrasm91]

Should I just create a new stub to not interfere with other tests?

This is fine.

[danieldietzler]

LGTM