dwyl / imgup

🌅 Effortless image uploads to AWS S3 with automatic resizing including REST API.
https://imgup.fly.dev/
105 stars 19 forks source link

Feat: Filenames on `S3` should be the `cid` of the content ... #62

Closed nelsonic closed 1 year ago

nelsonic commented 1 year ago

At present Unique File Names are using DateTime.utc_now():

image

This is does not take advantage of cid. The idea of cid is that when a file is uploaded the name of the file is based on the actual content of the file ... Therefore we need to read the binary of the file and feed that into cid/1 such that when the same file is uploaded again, it will have the same filename on S3. The reason we want to do this is simple: we avoid duplicates.

Todo

LuchoTurtle commented 1 year ago

With the changes introduced in https://github.com/dwyl/imgup/blob/2f51c88ddf03e75163dd8bbdae3aa050a5d9492a/lib/app/upload.ex#L22-L27, I believe this issue is no longer relevant and should be closed.

nelsonic commented 1 year ago

The upload.ex is only used for the API.

LuchoTurtle commented 1 year ago

Because the current code used in the LiveView make the upload from Javascript (followed by the File Upload guide in https://hexdocs.pm/phoenix_live_view/uploads.html), because it's been requested that the LiveView uses the upload/1 function that was implemented, I'm going to create a new page to use LiveView but use this function. I'm not going to refactor the code of the pre-existing LiveView because it would essentially be overhauled.

In order to not lose the previous work made, I'm going to create a new page that is "clientless", which uses the upload/1 function that was implemented.

nelsonic commented 1 year ago

Indeed the guide written by Chris https://github.com/phoenixframework/phoenix_live_view/blob/v0.19.3/guides/server/uploads.md#L1 uses the JS "direct" uploads method with signing. That is preferable in certain circumstances. If we can read the contents of the file on the device/browser we could use the JS implementation of CID to create the filename and thus continue using the direct upload approach. But I find that it requires considerably more code and isn't well tested. 💭

If you end up making a "clientless" version that uses the upload/1 function, please document + test as much as possible. 🙏

LuchoTurtle commented 1 year ago

The closed PR https://github.com/dwyl/imgup/pull/64 directly addresses this. It generates the CID from the contents of the file within the JS code. Because using JS to push the file was not initially intended, I closed off the PR. If relevant, I might open it back up again to finish it.

LuchoTurtle commented 1 year ago

And yes, I'm doing the "clientless" version now on a different live view page. I want to get this done before doing other issues (like the image classifier) so I can have a more solid foundation to work on it.

I'm branching off from https://github.com/dwyl/imgup/pull/86#pullrequestreview-1512612719 so I don't have merge conflicts later on 👌