Add storage module for JVM

GitLiveApp / firebase-java-sdk

A pure java port of the Firebase Android SDK

Apache License 2.0

16 stars 5 forks source link

Add storage module for JVM #4

Open jakubkinst opened 6 months ago

jakubkinst commented 6 months ago

I would really appreciate storage module implementation for the JVM target. Is there any particular blocker for this one? Thanks

nbransby commented 6 months ago

No blockers - ready for someone (such as yourself!) to pick up https://github.com/GitLiveApp/firebase-java-sdk?tab=readme-ov-file#building-and-contributing

CRamsan commented 3 weeks ago

I can help with this. :+1: Do you guys have a slack/discord for some quick questions? I wanted to run some questions before getting started. In the mean-time if something comes up, I can put it here.

CRamsan commented 3 weeks ago

My goal is to be able to use Firebase Storage for a Compose Desktop app. So this would be my approach.

As described here, I need to use the Android SDK. This SDK will be exposed as a JVM target with the help of Robolectric to provide any functionality that require Android-specific APIs.
- On a side-note, is this the long term plan?
Given that this set up is a bit challenging, I will need to add several unit tests to validate the functionality.
Once the java-sdk has been updated with JVM support for Storage, I will go to https://github.com/GitLiveApp/firebase-kotlin-sdk/blob/master/firebase-storage/src/jvmMain/kotlin/dev/gitlive/firebase/storage/storage.jvm.kt and wire up the JVM implementation.
Add some more tests.
Anything else?

nbransby commented 2 weeks ago

No that's exactly it 👍 you can post any issues you encounter here

CRamsan commented 2 weeks ago

I see that in the build.gradle.kts, we are exposing all the classes from all the jars.

 api(fileTree(mapOf("dir" to "build/jar", "include" to listOf("*.jar"), "exclude" to listOf("lifecycle-*"))))

Is this required or can we have it just use implementation? Otherwise, all the jars will be added to the classpath and can very easily cause runtime exceptions even for components that are not firebase related.

For example, today I spent an hour investigating a crash in my navigation code. The crash was due to a missing method inside the JB Navigation due to the firebase-java-sdk exposing it's own version of several androidx.* dependencies.

CRamsan commented 2 weeks ago

This is what I have so far #18 . I still need to do some more testing.

nbransby commented 2 weeks ago

Yes it's required because the library consumers client code compiles and run directly against these android libraries, the code inside this project is an implementation of the android system using the standard java libraries required by the firebase libraries.

CRamsan commented 2 weeks ago

I dont think we need all those classes though. Maybe it would be better to make it an include list for only the classes or packages that are required. Although I dont know how many those are :thinking: . Otherwise, it will be a problem now that a lot of the AndroidX libraries are becoming multi-platform, and there will be classes that are androidx which will be used in the JVM target as part of those libraries.

nbransby commented 2 weeks ago

Any library excluded from the jars will need to be added via a gradle implementation clause so the code will still compile and run

CRamsan commented 2 weeks ago

My priorities had to change so I will not be needing the storage SDK anymore. Here are the changes I had started, the library loads but I am getting a 400 response when trying to download any file. Hopefully someone can take over and finish the rest.

Java-sdk changes: #18 Kotlin-sdk changes: branch