Add metadata to Firebase Storage #514
Conversation
|
Thanks for this @deBasMan21. The API design should match the Android Firebase KTX SDK (https://firebase.google.com/docs/storage/android/upload-files?hl=en&authuser=0#add_file_metadata) so code written for android would still compile for KMP, you can read about this here: https://github.com/GitLiveApp/firebase-kotlin-sdk?tab=readme-ov-file#compatibility-with-the-official-firebase-android-sdk Also could you add some tests? |
|
@nbransby i added the same syntax so it will compile when you do a drop in replacement. As for tests, there are currently no tests yet for the whole storage module so i didn't add them yet. If you really require this pr to have tests, can you specify what you want to have tested? It is just an extra object that is passed to the function calls so i don't know what should be tested since there is no logic in there. |
|
@nbransby have you had time to look at my previous comment? |
|
Sorry for the late reply @deBasMan21 I guess the test should just confirm the metadata you set is equal to what you recieve back when you get it |
|
@nbransby Im looking at writing tests for this but in this PR there is no implementation for retrieving metadata from the client side, only for adding it when uploading a file. Ive been looking at a solution for retrieving metadata but its more complex than it seems because it returns a task on android, a completionhandler on ios and a promise on js so it will take some time for me to make it work. Its actually a completely different feature than the functionality proposed in this PR. Would it be ok if i work on this new functionality and create a seperate PR for it when it is finished, so this PR can be merged and used? I really need this in my current project and i am building a local version of this library to give me access to this functionality now :). Would be much appreciated. |
|
The task / completionhandler / promise thing is easy to handle as we do it everywhere in the sdk but you can just write a test to add metadata and assert it doesn't throw an exception for now |
…asMan21/firebase-kotlin-sdk into feat/firebaseStorageMetadata
|
@nbransby i added the tests you requested. I also added the putData method and getMetadata method so the tests deliver a little more value than only validating it won't crash. I think this pr is ready for review now, what do you think? |
|
@nbransby could you take a look at this pr again? |
|
@nbransby The android tests seem to be failing on the auth module while i didn't make any changes there. Locally all tests seem to work so i think it is a problem with running the firebase emulators in ci. Would this be blocking for this pr to merge? |
|
The android tests are flakey so we'll have to ignore them for now |
No description provided.