search

capawesome-team / capacitor-firebase

⚡️ Firebase plugins for Capacitor. Supports Android, iOS and the Web.
https://capawesome.io/plugins/firebase/
Apache License 2.0
410 stars 104 forks source link

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

Open mamillastre opened 5 months ago

mamillastre commented 5 months ago

Plugin(s)

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

@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 commented 5 months ago

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 commented 5 months ago

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.

robingenz commented 5 months ago

You're right, that's better.