Page MenuHomePhabricator

[native] Implement filesystem media cache
ClosedPublic

Authored by bartek on Mar 30 2023, 7:03 AM.
Tags
None
Referenced Files
F3516413: D7252.id24483.diff
Sun, Dec 22, 2:24 PM
F3516409: D7252.id24592.diff
Sun, Dec 22, 2:22 PM
Unknown Object (File)
Wed, Dec 18, 10:53 PM
Unknown Object (File)
Wed, Dec 4, 3:31 AM
Unknown Object (File)
Wed, Dec 4, 3:31 AM
Unknown Object (File)
Wed, Dec 4, 3:31 AM
Unknown Object (File)
Wed, Dec 4, 3:31 AM
Unknown Object (File)
Wed, Dec 4, 3:31 AM
Subscribers

Details

Summary

This diff implements native methods to manage media cache stored in filesystem: the tmp_dir/media-cache directory.

Depends on D7251

Test Plan

Tested together with parent diff - saved some files to cache and then retrieved it. It is possible to print the cache path and open it with Finder to inspect what files are inside.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

bartek held this revision as a draft.
bartek published this revision for review.Mar 30 2023, 9:30 AM
ashoat added inline comments.
lib/media/file-utils.js
203

Should the first param be a RegExp to make sure it doesn't replace other parts of the filename? Not sure this works, but something like

new RegExp(`.${extension}$`)
native/media/media-cache.js
1

Nit: add extra newline here

30

Nit: add newline here

33–34

Nit

69

Can we extract this RegExp object and assign it to a variable in the global scope to avoid recreating it every time?

This revision is now accepted and ready to land.Mar 30 2023, 4:35 PM
lib/media/file-utils.js
203

Good point

Please escape the . character in the RegExp before landing! Sorry I didn't notice this in my suggestion

lib/media/file-utils.js
203 ↗(On Diff #24483)

The . character here matches anything, so eg. if you mean to replace test.blah -> test`, you could accidentally replace test.zblah -> test..

I think we can escape the . character to avoid this

This revision was automatically updated to reflect the committed changes.