Lychee icon indicating copy to clipboard operation
Lychee copied to clipboard

Add upload of Zip, extract and continue

Open ildyria opened this issue 1 year ago • 2 comments

Add support of upload of single zip files.

:warning: this is quite experimental, I tested locally that it works, however there are some caveats to know about.

  1. The uploaded zip file will stay on your server unless it is properly extracted without faults (after which it is removed).
  2. Similarly the extracted files from the zip will stay on your server unless they are properly imported without fault.
  3. On import success, you will see in /path-to-lychee/storage/extract-jobs an empty folder prefixed with YmD so you know when it was created, making easier to clean up later.
  4. ONLY THE FIRST LEVEL OF THE ZIP IS IMPORTED. See explanation below.

In other words, if you have

Something_Stupid.zip
├── img1.jpg
├── img2.jpg
├── img3.jpg
└── SubFolder
    ├── img4.jpg
    ├── img5.jpg
    └── img6.jpg

Only img1.jpg, img2.jpg and img3.jpg will be imported in Something Stupid album (note that _ is automatically replaced by space) and deleted from /path-to-lychee/storage/extract-jobs/yyyymmddSomething_Stupid. The folder SubFolder will still be in /path-to-lychee/storage/extract-jobs/yyyymmddSomething_Stupid/SubFolder

ildyria avatar Jan 21 '24 18:01 ildyria

Codecov Report

Attention: Patch coverage is 0% with 89 lines in your changes are missing coverage. Please review.

Project coverage is 85.62%. Comparing base (41bea7a) to head (cf67c07).

Additional details and impacted files

codecov[bot] avatar Jan 21 '24 21:01 codecov[bot]

  • What's with the mangling and demangling of spaces? Why is it necessary?

Most of the time when a _ is in the file name when uploading a zip archive, it is meant as a space. I am planning later to make a renamer module which would make it nicer when uploading in batch.

  • Shouldn't this code extract the zip file and then simply invoke the existing "Import from Server" functionality that already correctly handles subfolders and such? Why reimplement a more limited version?

Import from Server is actually not so great when using from web interface. To the point where I removed it from the v5 GUI (still available via CLI).

The code is old and functionality needs rewriting. See TODO here: https://github.com/LycheeOrg/Lychee/blob/master/app/Actions/Import/Exec.php#L272

I am planning to revisit the import from server for a cleaner code using Jobs & pipelines, but I wanted to have a simple implementation of import via ZIP as POC.

  • Why is there no proper cleanup? Can't we wrap it all in a big try-catch and do the cleanup there?

It is slightly more complex than that. This is using laravel jobs which you could see as independent work unit which are dispatched on a queue and I am not sure which job will be picked first. More info on jobs here: https://laravel.com/docs/10.x/queues

  • Why is the empty folder left on the server even on success? Wouldn't a simple rmdir be enough to remove it?

See above, we don't know when all the jobs have been processed.

ildyria avatar Jan 22 '24 08:01 ildyria