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

feat(storage): add the getBlob and downloadFile methods #665

Open
3 of 13 tasks
mamillastre opened this issue Jul 2, 2024 · 7 comments
Open
3 of 13 tasks

feat(storage): add the getBlob and downloadFile methods #665

mamillastre opened this issue Jul 2, 2024 · 7 comments

Comments

@mamillastre
Copy link
Contributor

mamillastre commented Jul 2, 2024

Plugin(s)

  • Analytics
  • App
  • App Check
  • Authentication
  • Crashlytics
  • Cloud Firestore
  • Cloud Functions
  • Cloud Messaging
  • Cloud Storage
  • Performance
  • Remote Config

Current problem

The plugin allows to get the download URL of a file.
But if we protected a file with a rule, it may be useful to have another way and avoid generating a public link.

Preferred solution

In the firebase web sdk, we have the getBlob method to directly get the file as Blob.

On Android & iOS, the app can crash if we download the file in memory (see https://capawesome.io/blog/the-file-handling-guide-for-capacitor/).
For these native platforms, we can use the "download to local file" functionality.

So we can add a "getBlob()" method (only on web) and a "downloadFile()" method (only Android & iOS).

Alternative options

No response

Additional context

Firebase doc:

Before submitting

@robingenz
Copy link
Member

robingenz commented Jul 2, 2024

I like the idea, but using base64 leads to an out-of-memory exception with "large" files. For this reason, I always recommend interacting directly with the native file system (see this blog post). I'm not sure whether it's a good idea to make this API available to Capacitor developers. But feel free to create a PR and I'll take a look at it.

@mamillastre
Copy link
Contributor Author

Good point.
It can be useful for small files and to avoid the security issue of the download URL when a file is protected by a rule.
I stand by this feature for now. I may be working on this in the future.
Thank you.

@robingenz
Copy link
Member

It can be useful for small files and to avoid the security issue of the download URL when a file is protected by a rule.

Yes, that's a good point. I would probably merge the feature and wait and see what the feedback from the community is like.

@mamillastre
Copy link
Contributor Author

@robingenz, maybe an alternative solution to fix the security issue is to use the "download to a local file" functionality available on Android & iOS.

Android: https://firebase.google.com/docs/storage/android/download-files#download_to_a_local_file
iOS: https://firebase.google.com/docs/storage/ios/download-files#download_to_a_local_file

We can have a "getFile()" method that has a call signature similar to the "Filesystem.writeFile()" method's interfaces (WriteFileOptions & WriteFileResult).

What do you think of this solution ?

@robingenz
Copy link
Member

That is much better. However, I would probably prefer to call the method downloadFile(...) so that it matches uploadFile(...).

We can have a "getFile()" method that has a call signature similar to the "Filesystem.writeFile()" method's interfaces (WriteFileOptions & WriteFileResult).

I suggest the following for the call signature:

export interface DownloadFileOptions {
  /**
   * The full path to the file to download, including the file name.
   * 
   * @since 6.2.0
   */
  remotePath: string;
  /**
   * The uri to the local file, including the file name.
   * 
   * @since 6.2.0
   */
  localPath: string;
}

data is not required. Instead of using directory and path, I would rather require a uri (called localPath here) directly. Otherwise you always have to keep the types synchronized with the official Capacitor Filesystem plugin. And for now, I would not want to add a recursive option to keep it simple.

@mamillastre What do you think?

@mamillastre
Copy link
Contributor Author

Seems perfect !
With the help of the Filesystem plugin, we can manage the permission, create the directory (mkdir()) and get the uri (getUri()).

To be consistent with the Storage plugin and the Filesystem plugin, we can also have:

export interface DownloadFileOptions {
  /**
   * The full path to the file to download, including the file name.
   * 
   * @since 6.2.0
   */
  path: string;
  /**
   * The uri to the local file, including the file name.
   * 
   * @since 6.2.0
   */
  uri: string;
}

I will update this feature request.

For now, I do not have the time to work on this. But maybe later.

@mamillastre mamillastre changed the title feat(storage): add in memory download & from memory upload feat(storage): add the downloadFile method Jul 3, 2024
@mamillastre mamillastre changed the title feat(storage): add the downloadFile method feat(storage): add the getBlob and downloadFile methods Jul 3, 2024
@robingenz
Copy link
Member

You're right, that's better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants