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(firebase_storage): firebase storage Dart & Desktop #67

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Ahmed-elshorbagy
Copy link

starting firestorage dart/desktop implementation

@CLAassistant
Copy link

CLAassistant commented Apr 24, 2022

CLA assistant check
All committers have signed the CLA.

@pr-Mais pr-Mais changed the title FireStorage initial commit feat(firebase_storage): support for Firebase storage for Dart & Desktop Apr 25, 2022
@pr-Mais pr-Mais changed the title feat(firebase_storage): support for Firebase storage for Dart & Desktop feat(firebase_storage): firebase storage Dart & Desktop Apr 25, 2022
@Ahmed-elshorbagy Ahmed-elshorbagy marked this pull request as ready for review April 26, 2022 12:24
@Ahmed-elshorbagy
Copy link
Author

Ahmed-elshorbagy commented Apr 26, 2022

@pr-Mais @TimWhiting
I need help in starting the api part so i can continue from there
as rest api is still in beta but it is supported in firebase javascript sdk so what should i do or start with

@Salakar Salakar marked this pull request as draft April 26, 2022 14:41
@TimWhiting
Copy link
Collaborator

I would look at how the javascript is accessing it: https://github.com/firebase/firebase-js-sdk/blob/master/packages/storage/src/implementation/requests.ts

Not sure I would organize it the same as the javascript, but the functions in that file seems to be the api surface I would implement in an FirebaseStorageApiClient class.

@pr-Mais
Copy link
Contributor

pr-Mais commented Apr 26, 2022

As @TimWhiting said, the JS SDK was my reference in auth, if you look at the API layer in Auth you can take inspiration.
Additionally, we use the generated Firebase APIs instead of building it from scratch.

@Ahmed-elshorbagy
Copy link
Author

@pr-Mais @TimWhiting I have already done this but couldn't figure how to extend this by imported code from js sdk as some of functions are implemented twice with different approaches i have searched also the android firebase library so iam stuck at the start but i will try my best

@pr-Mais
Copy link
Contributor

pr-Mais commented Apr 27, 2022

I used to check the three SDKs for the same functionality (iOS, Android, and JS). There's a way each is implementing something following the practices for the language and framework, but the general idea should be the same.
You can start from the Dart API part in firebase_storage_web, then add an API layer under it, which will be in place of interop part in the web package.

@Ahmed-elshorbagy
Copy link
Author

Ahmed-elshorbagy commented May 2, 2022

sorry for bothering you but how can i test my implementationsof any api request is correct
thanks in advance for both dart /desktop packages "should i create them from scratch??"

@Ahmed-elshorbagy
Copy link
Author

how to get "'X-Firebase-Storage-Version'" token inside firebase app @pr-Mais @TimWhiting

@Salakar
Copy link
Member

Salakar commented May 9, 2022

Off-topic but I've just added the newly released firebasestorage:v1 api (invertase/dart_firebase_apis#3) to https://github.com/invertase/dart_firebase_apis

@pr-Mais
Copy link
Contributor

pr-Mais commented May 9, 2022

@Ahmed-elshorbagy sorry for delay. Could you reference a link to where is X-Firebase-Storage-Version mentioned?

Regarding testing, write test cases following FlutterFire tests for the Desktop package, and refer to the tests in auth for the Dart package. Also, make an example app in the Desktop package and test the functionalities you implement.

@Ahmed-elshorbagy
Copy link
Author

Ahmed-elshorbagy commented May 9, 2022

@pr-Mais https://github.com/firebase/firebase-js-sdk/blob/e34e98e73a72f77ee87d9005d6728402129deda9/packages/storage/src/implementation/request.ts#L249
this header is required form most app requests its linked to json file in app core package

@pr-Mais
Copy link
Contributor

pr-Mais commented May 9, 2022

I tracked it down here: https://github.com/firebase/firebase-js-sdk/blob/cdada6c68f9740d13dd6674bcb658e28e68253b6/packages/app/src/api.ts#L31-L51

So this header represents the current SDK version used by the app making the requests. On Dart, could be read from pubspec.yaml, will see the best way to get this done for all packages, I assume they all would have such a header. For now, you can manually set a constant with the initial version number hardcoded

@Ahmed-elshorbagy Ahmed-elshorbagy marked this pull request as ready for review May 24, 2022 22:33
@ghost
Copy link

ghost commented Jun 11, 2022

cloud.

@stMerlHin
Copy link

+1

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

Successfully merging this pull request may close these issues.

6 participants