-
-
Notifications
You must be signed in to change notification settings - Fork 301
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add upload of Zip, extract and continue #2229
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've got multiple questions...
- What's with the mangling and demangling of spaces? Why is it necessary?
- 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?
- Why is there no proper cleanup? Can't we wrap it all in a big try-catch and do the cleanup there?
- Why is the empty folder left on the server even on success? Wouldn't a simple
rmdir
be enough to remove it?
database/migrations/2024_01_21_182512_add_flag_support_simple_zip_upload.php
Outdated
Show resolved
Hide resolved
database/migrations/2024_01_21_182512_add_flag_support_simple_zip_upload.php
Outdated
Show resolved
Hide resolved
$baseNameWithoutExtension = substr($this->originalBaseName, 0, -4); | ||
|
||
// Save that one (is default if no existing folder found). | ||
$orignalName = str_replace(' ', '_', $baseNameWithoutExtension); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are you doing this replacement here? Does ZipArchive not handle spaces correctly or something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may, but I don't trust spaces in paths.
And given that we replace '_' by ' ' later in file names. I figured it would be idem potent.
Most of the time when a
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. 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.
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.
See above, we don't know when all the jobs have been processed. |
5545adc
to
3e78544
Compare
bc52805
to
23e2382
Compare
cf67c07
to
e244d47
Compare
e244d47
to
4961bb6
Compare
4961bb6
to
3cf748d
Compare
3cf748d
to
20a68d2
Compare
20a68d2
to
dfcc429
Compare
Add support of upload of single zip files.
/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.In other words, if you have
Only
img1.jpg
,img2.jpg
andimg3.jpg
will be imported inSomething 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