feat(Share): Generate zip file on the fly

[aarondoet]

Goal of this PR is to no longer generate zip files for all shares by default but rather generate them on the fly when a download is requested. The only reason for this to generate overhead over someone downloading the files one by one would be the compression - which one could get rid of.

[aarondoet]

Not completely finished yet, need to remove the zip generation and saving on upload and the isZipReady property. I have tested this already though, the API now generates the zip file when requested.

[adriaanh]

Would this also apply for the S3 storage option? As currently the S3 storage doesn't have the ZIP functionality.

[aarondoet]

I would assume that this should also work with S3. However I never used S3 and don't intend to, so I won't be able to verify.

[aarondoet]

1. I now always show the Download All button, even if there is just one file. That makes the UI consistent and because the ZIP can now also be generated for single files I don't see a reason not to do it. This is personal preference however, I can undo this change again.
2. The ZIP is now generated every time it is downloaded meaning the compression is also done every time. Personally I'd just remove the compression setting (not yet done though), with compression level 0 the ZIP generation only has negligible overhead compared to just downloading each file.

@stonith404 can you give me some input on this?

[aarondoet]

Is it enough to remove the translation keys for english and Crowdin will remove it for the others as well or do you have to remove them all manually?

[stonith404]

I would assume that this should also work with S3. However I never used S3 and don't intend to, so I won't be able to verify.

There is probably a way but I'm not using S3 as well so someone else would have to contribute to support S3.

I now always show the Download All button, even if there is just one file. That makes the UI consistent and because the ZIP can now also be generated for single files I don't see a reason not to do it. This is personal preference however, I can undo this change again.

Yeah that makes sense.

The ZIP is now generated every time it is downloaded meaning the compression is also done every time. Personally I'd just remove the compression setting (not yet done though), with compression level 0 the ZIP generation only has negligible overhead compared to just downloading each file.

Yes we can do that. If anyone misses the compression feature, they can submit a feature request, and we can consider reintroducing it.

Is it enough to remove the translation keys for english and Crowdin will remove it for the others as well or do you have to remove them all manually?

Crowdin will remove the strings from the other files.

[stonith404]

I would put the logic into the local.service.ts because file.service.ts is just a wrapper of local.service.ts and s3.service.ts.

With that it would also be easier to add the zip functionality to S3 later.

[adriaanh]

To me, it seems that it's not storage related and therefore the file.service.ts is the better place. It's reading all the related files from any of the storage backends, and put it in a ZIP. Before, you had the getZip in the storage specific layer, as there was a zip file already in the storage.

[stonith404]

I would say that the functionality there is storage related. Especially the call to fs.createReadStream would be handled differently with S3.

[stonith404]

I'm setting this to "Request changes". See my comment above.