Skip to content
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

Save AMI set zip file in permanent instead of temporary storage #39

Open
aksm opened this issue Nov 2, 2021 · 2 comments
Open

Save AMI set zip file in permanent instead of temporary storage #39

aksm opened this issue Nov 2, 2021 · 2 comments
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed question Further information is requested
Milestone

Comments

@aksm
Copy link
Contributor

aksm commented Nov 2, 2021

What?

Zip files for AMI sets were previously saved in permanent storage but are currently saved in temporary, e.g. /system/temporary?file=ami/files.zip instead of /sites/default/files/2021/11/files.zip.

Per @DiegoPino probably the following needs to be added somewhere:

$file->setPermanent();
$file->save();
// Add to file usage calculation.
\Drupal::service('file.usage')->add($file, 'my_module_name', 'file', $file->id());
@aksm aksm added the bug Something isn't working label Nov 2, 2021
@DiegoPino
Copy link
Member

@aksm @alliomeria ok, false alarm! Just did a lot of testing and the fact that its in tmp:// does not mean its not permanent. Drupal won't delete it automatically. I actually deleted the file (checked my terminal logs, it was me) when cleaning up.

I also have an inline/code documentation explaining why its going into temporary://ami and not public:// or private://
The reason is, that until the Entity is saved (the AMI) the file itself can not be accessed by the user (download link after uploading) because Drupal needs any file to be attached to an entity before allowing permission. Once the entity is saved all goes well.

So. The way it is works. But for safety I could move the file after saving to a more "secure" location if needed?
Let's talk this out, but at least its not broken, it was me messing up with the storage layer.

Hugs

@DiegoPino DiegoPino added enhancement New feature or request help wanted Extra attention is needed question Further information is requested and removed bug Something isn't working labels Nov 3, 2021
@DiegoPino
Copy link
Member

WE should revisit this. Also we have seen ZIP file name conflicts in the past

@DiegoPino DiegoPino added this to the 0.9.0 milestone Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants