refactor: Migrate Firebase Storage to nnbd #4753

Salakar · 2021-01-21T13:01:37Z

Description

NNBD migration work for Firebase Storage.

Related Issues

Replace this paragraph with a list of issues related to this PR from the issue database. Indicate, which of these issues are resolved or fixed by this PR. Note that you'll have to prefix the issue numbers with flutter/flutter#.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]).
This will ensure a smooth and quick review process. Updating the pubspec.yaml and changelogs is not required.

I read the Contributor Guide and followed the process outlined there for submitting PRs.
My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
All existing and new tests are passing.
I updated/added relevant documentation (doc comments with ///).
The analyzer (flutter analyze) does not report any problems on my PR.
I read and followed the Flutter Style Guide.
I signed the CLA.
I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
No, this is not a breaking change.

into @ehesp/storage-nndb # Conflicts: # packages/firebase_storage/firebase_storage/pubspec.yaml

google-cla · 2021-01-21T13:01:47Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

google-cla · 2021-01-21T13:08:45Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

google-cla · 2021-01-21T13:21:20Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

google-cla · 2021-02-04T12:21:22Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

russellwheatley · 2021-02-04T12:25:06Z

packages/firebase_storage/firebase_storage_web/lib/src/firebase_storage_web.dart

      : super(appInstance: app, bucket: bucket);

+  /// The js-interop layer for Firebase Storage
+  final storage_interop.Storage? webStorage;


This is nullable. This is the same as auth web. although it violate that principle you mentioned yesterday, @Salakar. It's only nullable to satisfy registerWith. I wonder if it is necessary.

russellwheatley · 2021-02-04T12:26:52Z

packages/firebase_storage/firebase_storage_web/lib/src/task_snapshot_web.dart

@@ -17,7 +15,7 @@ class TaskSnapshotWeb extends TaskSnapshotPlatform {
      ReferencePlatform ref, storage_interop.UploadTaskSnapshot snapshot)
      : _reference = ref,
        _snapshot = snapshot,
-        super(fbTaskStateToTaskState(snapshot.state), null);
+        super(fbTaskStateToTaskState(snapshot.state), {});


passed in empty Map literal otherwise it's an error. not sure if this is ideal.

russellwheatley · 2021-02-04T12:32:26Z

packages/firebase_storage/firebase_storage_web/lib/src/interop/storage.dart

+    } else {
+      return {};
+    }
+  }


I updated all this logic because this: dartify(jsObject.customMetadata) as Map can easily be null which didn't cause a static analysis error when casting, but if I ran it this code in dartpad, it did:

var map = null; var thing = (map as Map).cast<String, String>(); thing['stuff'] = "foo";

I also returned empty Map literal. If make the getter type nullable (Map<String, String>? get customMetadata), we also have to make the setter param nullable ( set customMetadata(Map<String, String>? m)). Didn't seem right, happy to change though.

packages/firebase_storage/firebase_storage_web/pubspec.yaml

google-cla · 2021-02-04T16:47:02Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

russellwheatley · 2021-02-04T16:55:53Z

packages/firebase_storage/firebase_storage/lib/src/firebase_storage.dart

-    // A bucket must exist at this point
-    // TODO(ehesp): Check whether `app.options.storageBucket` can be nullable post migration
-    if (bucket == null) {
+    if (bucket == null && app.options.storageBucket == null) {


I updated this code to be a bit more clear about what is happening. bucket and app.options.storageBucket are nullable. If both are null, throw an error.

_bucket initialization occurs here

google-cla · 2021-02-05T19:40:40Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

google-cla · 2021-02-08T16:57:58Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

russellwheatley · 2021-02-08T16:59:03Z

@googlebot I consent.

google-cla · 2021-02-08T16:59:19Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

Ehesp · 2021-02-08T17:04:56Z

@googlebot I consent.

packages/firebase_storage/firebase_storage/test/firebase_storage_test.dart

packages/firebase_storage/firebase_storage/example/lib/main.dart

packages/firebase_storage/firebase_storage/lib/src/firebase_storage.dart

packages/firebase_storage/firebase_storage/lib/src/reference.dart

packages/firebase_storage/firebase_storage/test/firebase_storage_test.dart

...base_storage/firebase_storage_platform_interface/lib/src/method_channel/utils/exception.dart

russellwheatley · 2021-02-10T17:03:57Z

I had to remove customMetadata as a param on SettableMetadata due to it causing this error on web e2e tests:

Expected a value of type 'Map<String, String>?', but got one of type 'IdentityMap<String, dynamic>'

This is the relevant commit.

Not really sure why it is casting to a different type, apparently no documentation on IdentityMap either. Either way, I couldn't figure it out within the time constraints, and I thought it more important to get this over the line. I've raised a ticket internally for this.

I also commented out a test, it is supposed to error but it doesn't. This just needs confirmation on expected behaviour, and is included as part of the ticket.

Internal ticket: INVERTASE/FF-29

…Flutter.podspec

Salakar

Some unused dependencies

packages/firebase_storage/firebase_storage_platform_interface/pubspec.yaml

packages/firebase_storage/firebase_storage/pubspec.yaml

Co-authored-by: Mike Diarmid <[email protected]>

russellwheatley · 2021-02-12T10:58:32Z

This PR: #5014 ought to be merged into this PR first to fix a number of issues described in the description.

…ings/errors (#5014)

packages/firebase_storage/firebase_storage/example/test_driver/reference_e2e.dart

Ehesp and others added 6 commits January 18, 2021 14:27

add linting rules

41ede7c

update pubspec to null safe versions

400ab59

fix linter warnings

bffd376

apply platform null safey migration

29be8a7

refactor: fix linting issues

328e619

Merge branch 'master' of https://github.com/FirebaseExtended/flutterfire

62cfc6f

into @ehesp/storage-nndb # Conflicts: # packages/firebase_storage/firebase_storage/pubspec.yaml

google-cla bot added the cla: no label Jan 21, 2021

Salakar changed the title ~~refactor: Migrate Firebase Storage to nnbd~~ refactor: Migrate Firebase Storage to nnbd Jan 21, 2021

refactor: fix formatting

f47df2b

refactor: bump firebase_core version on PI deps

4f6767c

russellwheatley added 2 commits February 3, 2021 10:18

Merge branch 'master' into @ehesp/storage-nndb

1deee8d

refactor(storage_web): migrate storage_web package NNBD

1de6785

russellwheatley reviewed Feb 4, 2021

View reviewed changes

packages/firebase_storage/firebase_storage_web/pubspec.yaml Show resolved Hide resolved

russellwheatley reviewed Feb 4, 2021

View reviewed changes

packages/firebase_storage/firebase_storage_web/pubspec.yaml Show resolved Hide resolved

russellwheatley reviewed Feb 4, 2021

View reviewed changes

packages/firebase_storage/firebase_storage_web/pubspec.yaml Outdated Show resolved Hide resolved

russellwheatley added 2 commits February 4, 2021 16:27

refactor(storage): NNBD rework for main package

8050fc6

chore(storage): null safe packages

a432bb9

russellwheatley reviewed Feb 4, 2021

View reviewed changes

russellwheatley added 2 commits February 5, 2021 16:10

chore(storage):upgrade packages

16c385f

tests(storage): mock tests passing for storage main pkg

efeb476

tests(storage): platform interface test fixes

9789eba

russellwheatley requested review from Ehesp and kroikie as code owners February 8, 2021 16:57

google-cla bot added cla: yes and removed cla: no labels Feb 8, 2021

Ehesp reviewed Feb 10, 2021

View reviewed changes

packages/firebase_storage/firebase_storage/test/firebase_storage_test.dart Outdated Show resolved Hide resolved

rrousselGit reviewed Feb 10, 2021

View reviewed changes

packages/firebase_storage/firebase_storage/example/lib/main.dart Outdated Show resolved Hide resolved

rrousselGit approved these changes Feb 10, 2021

View reviewed changes

russellwheatley added 6 commits February 10, 2021 15:45

tests(storage_web): comment out broken tests for NNBD

44c0110

format: run formatter

c7fceb2

chore(storage): NNBD PR review updates

826c632

tests(storage): revert back to late for tests

b4eef7f

test(storage): make param dynamic to stop analysis err

109efaa

format: run formatter

dcc0d57

russellwheatley and others added 2 commits February 10, 2021 17:14

chore(storage): rm analyze warning

ccf9329

Restore packages/cloud_firestore/cloud_firestore/example/ios/Flutter/…

5f8fdda

…Flutter.podspec

Salakar commented Feb 11, 2021

View reviewed changes

packages/firebase_storage/firebase_storage_platform_interface/pubspec.yaml Outdated Show resolved Hide resolved

packages/firebase_storage/firebase_storage/pubspec.yaml Outdated Show resolved Hide resolved

russellwheatley and others added 2 commits February 11, 2021 11:20

remove pedantic dependency

f1c4073

Co-authored-by: Mike Diarmid <[email protected]>

chore(storage): add anlysis options to example

cf8325c

chore: mockito dependency updates, dependency changes & analysis warn…

0486583

…ings/errors (#5014)

Ehesp reviewed Feb 16, 2021

View reviewed changes

packages/firebase_storage/firebase_storage/example/test_driver/reference_e2e.dart Show resolved Hide resolved

Ehesp approved these changes Feb 18, 2021

View reviewed changes

russellwheatley merged commit dc609a7 into master Feb 18, 2021

russellwheatley deleted the @ehesp/storage-nndb branch February 18, 2021 11:45

firebase locked and limited conversation to collaborators Mar 21, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: Migrate Firebase Storage to nnbd #4753

refactor: Migrate Firebase Storage to nnbd #4753

Salakar commented Jan 21, 2021 •

edited by russellwheatley

Loading

google-cla bot commented Jan 21, 2021

google-cla bot commented Jan 21, 2021

google-cla bot commented Jan 21, 2021

google-cla bot commented Feb 4, 2021

russellwheatley Feb 4, 2021

russellwheatley Feb 4, 2021 •

edited

Loading

russellwheatley Feb 4, 2021

google-cla bot commented Feb 4, 2021

russellwheatley Feb 4, 2021

google-cla bot commented Feb 5, 2021

google-cla bot commented Feb 8, 2021

russellwheatley commented Feb 8, 2021

google-cla bot commented Feb 8, 2021

Ehesp commented Feb 8, 2021

russellwheatley commented Feb 10, 2021 •

edited

Loading

Salakar left a comment

russellwheatley commented Feb 12, 2021

refactor: Migrate Firebase Storage to nnbd #4753

refactor: Migrate Firebase Storage to nnbd #4753

Conversation

Salakar commented Jan 21, 2021 • edited by russellwheatley Loading

Description

Related Issues

Checklist

Breaking Change

google-cla bot commented Jan 21, 2021

google-cla bot commented Jan 21, 2021

google-cla bot commented Jan 21, 2021

google-cla bot commented Feb 4, 2021

russellwheatley Feb 4, 2021

Choose a reason for hiding this comment

russellwheatley Feb 4, 2021 • edited Loading

Choose a reason for hiding this comment

russellwheatley Feb 4, 2021

Choose a reason for hiding this comment

google-cla bot commented Feb 4, 2021

russellwheatley Feb 4, 2021

Choose a reason for hiding this comment

google-cla bot commented Feb 5, 2021

google-cla bot commented Feb 8, 2021

russellwheatley commented Feb 8, 2021

google-cla bot commented Feb 8, 2021

Ehesp commented Feb 8, 2021

russellwheatley commented Feb 10, 2021 • edited Loading

Salakar left a comment

Choose a reason for hiding this comment

russellwheatley commented Feb 12, 2021

Salakar commented Jan 21, 2021 •

edited by russellwheatley

Loading

russellwheatley Feb 4, 2021 •

edited

Loading

russellwheatley commented Feb 10, 2021 •

edited

Loading