From 19ee62c33f34278dac082c11bf7574785e60abb5 Mon Sep 17 00:00:00 2001 From: Russell Wheatley Date: Thu, 7 Apr 2022 14:19:48 +0100 Subject: [PATCH] fix(firebase_storage): Fix `UploadTask.cancel()` so that it completes when called. (#8417) --- .../storage/FlutterFirebaseStorageTask.java | 20 +----- tests/test_driver/driver_e2e.dart | 7 ++- .../firebase_storage/instance_e2e.dart | 2 - .../firebase_storage/reference_e2e.dart | 61 ++++++++++--------- 4 files changed, 40 insertions(+), 50 deletions(-) diff --git a/packages/firebase_storage/firebase_storage/android/src/main/java/io/flutter/plugins/firebase/storage/FlutterFirebaseStorageTask.java b/packages/firebase_storage/firebase_storage/android/src/main/java/io/flutter/plugins/firebase/storage/FlutterFirebaseStorageTask.java index 8928d928ea48..ca2703509ee6 100644 --- a/packages/firebase_storage/firebase_storage/android/src/main/java/io/flutter/plugins/firebase/storage/FlutterFirebaseStorageTask.java +++ b/packages/firebase_storage/firebase_storage/android/src/main/java/io/flutter/plugins/firebase/storage/FlutterFirebaseStorageTask.java @@ -35,7 +35,6 @@ class FlutterFirebaseStorageTask { private final StorageMetadata metadata; private final Object pauseSyncObject = new Object(); private final Object resumeSyncObject = new Object(); - private final Object cancelSyncObject = new Object(); private StorageTask storageTask; private Boolean destroyed = false; @@ -152,10 +151,6 @@ void destroy() { } } - synchronized (cancelSyncObject) { - cancelSyncObject.notifyAll(); - } - synchronized (pauseSyncObject) { pauseSyncObject.notifyAll(); } @@ -200,20 +195,7 @@ Task resume() { } Task cancel() { - return Tasks.call( - FlutterFirebasePlugin.cachedThreadPool, - () -> { - synchronized (cancelSyncObject) { - boolean canceled = storageTask.cancel(); - if (!canceled) return false; - try { - cancelSyncObject.wait(); - } catch (InterruptedException e) { - return false; - } - return true; - } - }); + return Tasks.call(FlutterFirebasePlugin.cachedThreadPool, () -> storageTask.cancel()); } void startTaskWithMethodChannel(@NonNull MethodChannel channel) throws Exception { diff --git a/tests/test_driver/driver_e2e.dart b/tests/test_driver/driver_e2e.dart index a603f03a9fb1..059d26373f67 100644 --- a/tests/test_driver/driver_e2e.dart +++ b/tests/test_driver/driver_e2e.dart @@ -3,6 +3,7 @@ // BSD-style license that can be found in the LICENSE file. import 'package:drive/drive.dart' as drive; +import 'package:flutter/foundation.dart'; import 'cloud_functions/cloud_functions_e2e.dart' as cloud_functions; import 'firebase_auth/firebase_auth_e2e.dart' as firebase_auth; import 'firebase_core/firebase_core_e2e.dart' as firebase_core; @@ -22,6 +23,7 @@ import 'firebase_ml_model_downloader/firebase_ml_model_downloader_e2e.dart' as firebase_ml_model_downloader; import 'firebase_remote_config/firebase_remote_config_e2e.dart' as firebase_remote_config; +import 'firebase_storage/firebase_storage_e2e.dart' as firebase_storage; void setupTests() { // Core first. @@ -29,7 +31,10 @@ void setupTests() { // All other tests. firebase_auth.setupTests(); cloud_functions.setupTests(); - // firebase_storage.setupTests(); + if (defaultTargetPlatform == TargetPlatform.android || kIsWeb) { + // TODO(russellwheatley): Pending release of latest firebase-tools with Storage emulator fix. + firebase_storage.setupTests(); + } firebase_database.setupTests(); firebase_app_check.setupTests(); firebase_messaging.setupTests(); diff --git a/tests/test_driver/firebase_storage/instance_e2e.dart b/tests/test_driver/firebase_storage/instance_e2e.dart index 570923edd682..eb69e40a0f96 100644 --- a/tests/test_driver/firebase_storage/instance_e2e.dart +++ b/tests/test_driver/firebase_storage/instance_e2e.dart @@ -15,8 +15,6 @@ void setupInstanceTests() { await Firebase.initializeApp( options: DefaultFirebaseOptions.currentPlatform, ); - await FirebaseStorage.instance - .useStorageEmulator(testEmulatorHost, testEmulatorPort); storage = FirebaseStorage.instance; secondaryApp = await testInitializeSecondaryApp(); }); diff --git a/tests/test_driver/firebase_storage/reference_e2e.dart b/tests/test_driver/firebase_storage/reference_e2e.dart index bc6ef605888d..69071fd2675b 100644 --- a/tests/test_driver/firebase_storage/reference_e2e.dart +++ b/tests/test_driver/firebase_storage/reference_e2e.dart @@ -220,41 +220,46 @@ void setupReferenceTests() { expect(result.prefixes.length, greaterThan(0)); }); - group('putData', () { - test('uploads a file with buffer', () async { - List list = utf8.encode(kTestString); + group( + 'putData', + () { + test('uploads a file with buffer', () async { + List list = utf8.encode(kTestString); - Uint8List data = Uint8List.fromList(list); + Uint8List data = Uint8List.fromList(list); - final Reference ref = storage.ref('flutter-tests').child('flt-ok.txt'); + final Reference ref = + storage.ref('flutter-tests').child('flt-ok.txt'); - final TaskSnapshot complete = await ref.putData( - data, - SettableMetadata( - contentLanguage: 'en', - ), - ); + final TaskSnapshot complete = await ref.putData( + data, + SettableMetadata( + contentLanguage: 'en', + ), + ); - expect(complete.metadata?.size, kTestString.length); - // Metadata isn't saved on objects when using the emulator which fails test - // expect(complete.metadata?.contentLanguage, 'en'); - }); + expect(complete.metadata?.size, kTestString.length); + // Metadata isn't saved on objects when using the emulator which fails test + // expect(complete.metadata?.contentLanguage, 'en'); + }); - //TODO(pr-mais): causes the emulator to crash - // test('errors if permission denied', () async { - // List list = utf8.encode('hello world'); - // Uint8List data = Uint8List.fromList(list); + //TODO(pr-mais): causes the emulator to crash + // test('errors if permission denied', () async { + // List list = utf8.encode('hello world'); + // Uint8List data = Uint8List.fromList(list); - // final Reference ref = storage.ref('/uploadNope.jpeg'); + // final Reference ref = storage.ref('/uploadNope.jpeg'); - // await expectLater( - // () => ref.putData(data), - // throwsA(isA() - // .having((e) => e.code, 'code', 'unauthorized') - // .having((e) => e.message, 'message', - // 'User is not authorized to perform the desired action.'))); - // }); - }); + // await expectLater( + // () => ref.putData(data), + // throwsA(isA() + // .having((e) => e.code, 'code', 'unauthorized') + // .having((e) => e.message, 'message', + // 'User is not authorized to perform the desired action.'))); + // }); + }, + skip: kIsWeb, + ); group('putBlob', () { test(