From a68cbbd4cdcbf2b7fc073cab1922c37760b9e04f Mon Sep 17 00:00:00 2001 From: Dennis Kugelmann Date: Wed, 26 Feb 2020 17:32:08 +0100 Subject: [PATCH 1/8] Fix side-effects of instance and doubleInstance tests --- .../lib/src/remote_config.dart | 9 +++++---- .../test/firebase_remote_config_test.dart | 13 +++++++++---- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/packages/firebase_remote_config/lib/src/remote_config.dart b/packages/firebase_remote_config/lib/src/remote_config.dart index 161a2b5b1447..b1f7df0c48ec 100644 --- a/packages/firebase_remote_config/lib/src/remote_config.dart +++ b/packages/firebase_remote_config/lib/src/remote_config.dart @@ -28,14 +28,15 @@ class RemoteConfig extends ChangeNotifier { LastFetchStatus get lastFetchStatus => _lastFetchStatus; RemoteConfigSettings get remoteConfigSettings => _remoteConfigSettings; - static Completer _instanceCompleter = Completer(); + @visibleForTesting + static Completer instanceCompleter = Completer(); /// Gets the instance of RemoteConfig for the default Firebase app. static Future get instance async { - if (!_instanceCompleter.isCompleted) { - _instanceCompleter.complete(await _getRemoteConfigInstance()); + if (!instanceCompleter.isCompleted) { + instanceCompleter.complete(await _getRemoteConfigInstance()); } - return _instanceCompleter.future; + return instanceCompleter.future; } static Future _getRemoteConfigInstance() async { diff --git a/packages/firebase_remote_config/test/firebase_remote_config_test.dart b/packages/firebase_remote_config/test/firebase_remote_config_test.dart index 937ac98a5d8d..c162517215b8 100644 --- a/packages/firebase_remote_config/test/firebase_remote_config_test.dart +++ b/packages/firebase_remote_config/test/firebase_remote_config_test.dart @@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:async'; + import 'package:firebase_remote_config/firebase_remote_config.dart'; import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -40,6 +42,10 @@ void main() { }); }); + tearDown(() { + RemoteConfig.instanceCompleter = Completer(); + }); + test('instance', () async { final RemoteConfig remoteConfig = await RemoteConfig.instance; expect( @@ -59,10 +65,9 @@ void main() { RemoteConfig.instance, RemoteConfig.instance, ]; - Future.wait(futures).then((List remoteConfigs) { - // Check that both returned Remote Config instances are the same. - expect(remoteConfigs[0], remoteConfigs[1]); - }); + final List remoteConfigs = await Future.wait(futures); + // Check that both returned Remote Config instances are the same. + expect(remoteConfigs[0], remoteConfigs[1]); }); }); From 29d4844160355f0927133b3867e262923c545f4a Mon Sep 17 00:00:00 2001 From: Dennis Kugelmann Date: Wed, 26 Feb 2020 17:32:45 +0100 Subject: [PATCH 2/8] Fix Future already completed error --- .../firebase_remote_config/lib/src/remote_config.dart | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/firebase_remote_config/lib/src/remote_config.dart b/packages/firebase_remote_config/lib/src/remote_config.dart index b1f7df0c48ec..4c9995ed5602 100644 --- a/packages/firebase_remote_config/lib/src/remote_config.dart +++ b/packages/firebase_remote_config/lib/src/remote_config.dart @@ -25,7 +25,9 @@ class RemoteConfig extends ChangeNotifier { RemoteConfigSettings _remoteConfigSettings; DateTime get lastFetchTime => _lastFetchTime; + LastFetchStatus get lastFetchStatus => _lastFetchStatus; + RemoteConfigSettings get remoteConfigSettings => _remoteConfigSettings; @visibleForTesting @@ -34,7 +36,11 @@ class RemoteConfig extends ChangeNotifier { /// Gets the instance of RemoteConfig for the default Firebase app. static Future get instance async { if (!instanceCompleter.isCompleted) { - instanceCompleter.complete(await _getRemoteConfigInstance()); + try { + instanceCompleter.complete(await _getRemoteConfigInstance()); + } on StateError catch (error) { + if (error.message != "Future already completed") rethrow; + } } return instanceCompleter.future; } From 04b13a580ca652af76323a70c2714324b8f98c27 Mon Sep 17 00:00:00 2001 From: Dennis Kugelmann Date: Wed, 26 Feb 2020 17:34:50 +0100 Subject: [PATCH 3/8] Additional check method channel was called for doubleInstance test --- .../test/firebase_remote_config_test.dart | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/firebase_remote_config/test/firebase_remote_config_test.dart b/packages/firebase_remote_config/test/firebase_remote_config_test.dart index c162517215b8..f5965761191b 100644 --- a/packages/firebase_remote_config/test/firebase_remote_config_test.dart +++ b/packages/firebase_remote_config/test/firebase_remote_config_test.dart @@ -44,6 +44,8 @@ void main() { tearDown(() { RemoteConfig.instanceCompleter = Completer(); + // empty log for next tests + log.removeWhere((_) => true); }); test('instance', () async { @@ -68,6 +70,9 @@ void main() { final List remoteConfigs = await Future.wait(futures); // Check that both returned Remote Config instances are the same. expect(remoteConfigs[0], remoteConfigs[1]); + // Check that method RemoteConfig#instance was called at least once + expect(log.length, isPositive); + expect(log[0], isMethodCall('RemoteConfig#instance', arguments: null)); }); }); From 0184d20982a79437ea49b28385ef7b8bd3efeda7 Mon Sep 17 00:00:00 2001 From: Dennis Kugelmann Date: Wed, 26 Feb 2020 17:54:01 +0100 Subject: [PATCH 4/8] Use single quote strings --- packages/firebase_remote_config/lib/src/remote_config.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firebase_remote_config/lib/src/remote_config.dart b/packages/firebase_remote_config/lib/src/remote_config.dart index 4c9995ed5602..aa7a6c307a6b 100644 --- a/packages/firebase_remote_config/lib/src/remote_config.dart +++ b/packages/firebase_remote_config/lib/src/remote_config.dart @@ -39,7 +39,7 @@ class RemoteConfig extends ChangeNotifier { try { instanceCompleter.complete(await _getRemoteConfigInstance()); } on StateError catch (error) { - if (error.message != "Future already completed") rethrow; + if (error.message != 'Future already completed') rethrow; } } return instanceCompleter.future; From 467e6b8335829af6f3b4996f906bd8941b719d7c Mon Sep 17 00:00:00 2001 From: Dennis Kugelmann Date: Wed, 26 Feb 2020 23:41:23 +0100 Subject: [PATCH 5/8] Use preferred if check over try/catch --- .../firebase_remote_config/lib/src/remote_config.dart | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/firebase_remote_config/lib/src/remote_config.dart b/packages/firebase_remote_config/lib/src/remote_config.dart index aa7a6c307a6b..d5ec2f02c8b7 100644 --- a/packages/firebase_remote_config/lib/src/remote_config.dart +++ b/packages/firebase_remote_config/lib/src/remote_config.dart @@ -36,10 +36,11 @@ class RemoteConfig extends ChangeNotifier { /// Gets the instance of RemoteConfig for the default Firebase app. static Future get instance async { if (!instanceCompleter.isCompleted) { - try { - instanceCompleter.complete(await _getRemoteConfigInstance()); - } on StateError catch (error) { - if (error.message != 'Future already completed') rethrow; + if (!instanceCompleter.isCompleted) { + final _remoteConfigInstance = await _getRemoteConfigInstance(); + if (!instanceCompleter.isCompleted) { + instanceCompleter.complete(_remoteConfigInstance); + } } } return instanceCompleter.future; From 2bcd6f7164b3e3d36f196a0a5d32afa0f8de8742 Mon Sep 17 00:00:00 2001 From: Dennis Kugelmann Date: Wed, 26 Feb 2020 23:43:06 +0100 Subject: [PATCH 6/8] Remove double if statement --- packages/firebase_remote_config/lib/src/remote_config.dart | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/firebase_remote_config/lib/src/remote_config.dart b/packages/firebase_remote_config/lib/src/remote_config.dart index d5ec2f02c8b7..9bd461eb6352 100644 --- a/packages/firebase_remote_config/lib/src/remote_config.dart +++ b/packages/firebase_remote_config/lib/src/remote_config.dart @@ -36,11 +36,9 @@ class RemoteConfig extends ChangeNotifier { /// Gets the instance of RemoteConfig for the default Firebase app. static Future get instance async { if (!instanceCompleter.isCompleted) { + final _remoteConfigInstance = await _getRemoteConfigInstance(); if (!instanceCompleter.isCompleted) { - final _remoteConfigInstance = await _getRemoteConfigInstance(); - if (!instanceCompleter.isCompleted) { - instanceCompleter.complete(_remoteConfigInstance); - } + instanceCompleter.complete(_remoteConfigInstance); } } return instanceCompleter.future; From 1c38ea43ce1ee2892aa1570ac98b52b505b3a7ee Mon Sep 17 00:00:00 2001 From: Dennis Kugelmann Date: Thu, 27 Feb 2020 00:16:51 +0100 Subject: [PATCH 7/8] Update version and CHANGELOG --- packages/firebase_remote_config/CHANGELOG.md | 5 +++++ packages/firebase_remote_config/pubspec.yaml | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/firebase_remote_config/CHANGELOG.md b/packages/firebase_remote_config/CHANGELOG.md index 72c328ab2e07..5a21a7ea55ea 100644 --- a/packages/firebase_remote_config/CHANGELOG.md +++ b/packages/firebase_remote_config/CHANGELOG.md @@ -1,3 +1,8 @@ +## 0.3.0+2 + +* Fix `Bad state: Future already completed` error when initially + calling `RemoteConfig.instance` multiple times in parallel. + ## 0.3.0+1 * Remove the deprecated `author:` field from pubspec.yaml diff --git a/packages/firebase_remote_config/pubspec.yaml b/packages/firebase_remote_config/pubspec.yaml index c1e4a361bd80..21aec2f70c48 100644 --- a/packages/firebase_remote_config/pubspec.yaml +++ b/packages/firebase_remote_config/pubspec.yaml @@ -2,7 +2,7 @@ name: firebase_remote_config description: Flutter plugin for Firebase Remote Config. Update your application look and feel and behaviour without re-releasing. homepage: https://github.com/FirebaseExtended/flutterfire/tree/master/packages/firebase_remote_config -version: 0.3.0+1 +version: 0.3.0+2 dependencies: flutter: From f494d5dc56602abe569f0838b09412c2e19a61eb Mon Sep 17 00:00:00 2001 From: Dennis Kugelmann Date: Thu, 27 Feb 2020 00:19:39 +0100 Subject: [PATCH 8/8] Update version --- packages/firebase_remote_config/pubspec.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firebase_remote_config/pubspec.yaml b/packages/firebase_remote_config/pubspec.yaml index 21aec2f70c48..64733d3775f0 100644 --- a/packages/firebase_remote_config/pubspec.yaml +++ b/packages/firebase_remote_config/pubspec.yaml @@ -2,7 +2,7 @@ name: firebase_remote_config description: Flutter plugin for Firebase Remote Config. Update your application look and feel and behaviour without re-releasing. homepage: https://github.com/FirebaseExtended/flutterfire/tree/master/packages/firebase_remote_config -version: 0.3.0+2 +version: 0.3.0+3 dependencies: flutter: