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.

Sign up for GitHub

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: Revised Usage of Remote Parameter Fetcher #18

Merged
merged 5 commits into from
Nov 12, 2023
+272 −202
Merged

feat: Revised Usage of Remote Parameter Fetcher #18

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0240a8d
fix: activateAndRefetch method to call
naipaka Nov 12, 2023
ada9fab
fix: Refactor onConfigUpdated to Property for Consistent Stream Instance
naipaka Nov 12, 2023
cdee8dc
refactor: Refactor RemoteParameterFetcher to use
naipaka Nov 12, 2023
c784440
fix: Fix remote_parameter_fetcher example
naipaka Nov 12, 2023
2ed9fdb
refactor: Implement Use of ValueChanged
naipaka Nov 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 14 additions & 23 deletions packages/flutterfire_remote_parameter_fetcher/example/lib/main.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,26 +27,28 @@ class MainApp extends StatefulWidget {
class _MainAppState extends State<MainApp> {
late final rpf = widget.rpf;

late final _intRemoteParameter = rpf.getIntParameter('int_parameter');
late RemoteParameter<int> _intRemoteParameter;

late int _intParameterValue;

void listen(int value) {
debugPrint('remoteParameter value changed: $value');
_intParameterValue = value;
}

@override
void initState() {
super.initState();
_intRemoteParameter = rpf.getIntParameter(
'int_parameter',
onConfigUpdated: (value) {
debugPrint('remoteParameter value changed: $value');
setState(() {
_intParameterValue = value;
});
},
);
_intParameterValue = _intRemoteParameter.value;
_intRemoteParameter.addListener(listen);
}

@override
void dispose() {
// TODO(riscait): adding
// remoteParameter.removeListener(listen);
Future<void> dispose() async {
await _intRemoteParameter.dispose();
Comment on lines +50 to +51
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

badge
Thanks for the implementation!

super.dispose();
}

Expand All @@ -57,19 +59,8 @@ class _MainAppState extends State<MainApp> {
appBar: AppBar(title: const Text('FlutterFireRemoteParameterFetcher')),
body: Padding(
padding: const EdgeInsets.all(32),
child: Column(
mainAxisAlignment: MainAxisAlignment.center,
crossAxisAlignment: CrossAxisAlignment.stretch,
children: [
Text('RemoteParameter value: $_intParameterValue'),
const SizedBox(height: 16),
FilledButton(
onPressed: () async {
await _intRemoteParameter.activateAndRefetch();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

badge
Does this mean that it is no longer necessary to expose methods for forced refresh such as activateAndRefetch to the outside world?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, as of now, onConfigUpdate suffices, so there's no need to expose RemoteParameter externally anymore!
It seems that onConfigUpdate is also triggered when returning from the background, which was the intended use case.
In RemoteParameterFetcher, fetchAndActivate can still be used, so if re-fetching and activation are needed, we will continue to use that method.

},
child: const Text('Activate and refetch'),
),
],
child: Center(
child: Text('RemoteParameter value: $_intParameterValue'),
),
),
),
Expand Down
1 change: 1 addition & 0 deletions packages/flutterfire_remote_parameter_fetcher/lib/remote_parameter_fetcher.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/// Remote Parameter Fetcher
library remote_parameter_fetcher;

export '../src/remote_parameter.dart';
export '../src/remote_parameter_fetcher.dart';
53 changes: 9 additions & 44 deletions packages/flutterfire_remote_parameter_fetcher/lib/src/remote_parameter.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,54 +5,19 @@ import 'dart:async';
class RemoteParameter<T> {
RemoteParameter({
required T value,
required this.onConfigUpdated,
required this.activateAndRefetch,
}) : _value = value;
required StreamSubscription<void> subscription,
}) : _value = value,
_subscription = subscription;

/// A Stream of updated parameter information.
Stream<void> onConfigUpdated;

/// A function that will activate the fetched config and refetch the value.
/// This is useful for when you want to force a refetch of the value.
final Future<T> Function() activateAndRefetch;
/// The subscription to the stream of updated parameter information.
final StreamSubscription<void> _subscription;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By passing subscription as an argument, it has become unnecessary for the caller of the class to be conscious of adding or removing listeners!


/// The current value of the parameter.
T get value => _value;
T _value;

final List<void Function(T value)> _listeners = [];

late StreamSubscription<void> _subscription;

/// Add a listener to be notified when the value changes.
/// Executed when the remote value is updated.
void addListener(void Function(T value) listener) {
_listeners.add(listener);

if (_listeners.length == 1) {
// When the listener is added for the first time,
// monitor the onConfigUpdated stream.
_subscription = onConfigUpdated.listen((_) async {
_value = await activateAndRefetch();
_notifyListeners();
});
}
}

/// Remove a listener.
Future<void> removeListener(void Function(T value) listener) async {
_listeners.remove(listener);

if (_listeners.isEmpty) {
// When the last listener is removed,
// cancel the subscription to the onConfigUpdated stream.
await _subscription.cancel();
}
}
final T _value;

void _notifyListeners() {
for (final listener in _listeners) {
listener(_value);
}
/// Closes the [RemoteParameter].
Future<void> dispose() async {
await _subscription.cancel();
}
}
126 changes: 67 additions & 59 deletions packages/flutterfire_remote_parameter_fetcher/lib/src/remote_parameter_fetcher.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import 'package:meta/meta.dart';

import 'remote_parameter.dart';

typedef ValueChanged<T> = void Function(T value);

/// A class that wraps Remote Config.
/// Its role is to "fetch the configured parameters from remote and provide
/// them".
Expand Down Expand Up @@ -49,9 +51,17 @@ class RemoteParameterFetcher {
}

/// Provide a Stream of updated parameter information.
///
/// NOTE: In the method form, each call inadvertently creates
/// a different Stream, resulting in only the latest one being functional.
/// To address this issue, we use a property form to ensure a unique and
/// consistent Stream for each instance.
@visibleForTesting
Stream<RemoteConfigUpdate> get onConfigUpdated {
return _rc.onConfigUpdated;
late final Stream<RemoteConfigUpdate> onConfigUpdated = _rc.onConfigUpdated;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As noted in the comment, the original method format created different Streams each time it was called, resulting in only the latest Stream functioning effectively.
Therefore, we changed it to hold just one Stream per instance.
This ensures that once a Stream is created, it is continuously used for that instance, guaranteeing efficient and consistent operation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

badge
I see, I hadn't even thought about the behavior of getter and late final!


/// Filter the Stream of updated parameter information by key.
Stream<RemoteConfigUpdate> filteredOnConfigUpdated(String key) {
return onConfigUpdated.where((config) => config.updatedKeys.contains(key));
}

@visibleForTesting
Expand Down Expand Up @@ -104,103 +114,101 @@ class RemoteParameterFetcher {
}

/// Returns a [RemoteParameter] of type [String].
RemoteParameter<String> getStringParameter(String key) {
RemoteParameter<String> getStringParameter(
String key, {
required ValueChanged<String> onConfigUpdated,
}) {
Comment on lines +117 to +120
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of using addListener, we changed the approach to pass the callback for when onConfigUpdated is triggered as an argument. This allows for a more streamlined implementation, as shown in the following example using Riverpod.

@riverpod
String stringParameter(StringParameterRef ref) {
  final fetcher = ref.watch(remoteParameterFetcherProvider);
  final parameter = fetcher.getStringParameter(
    stringParameterKey,
    onConfigUpdated: (value) => ref.state = value,
  );
  ref.onDispose(parameter.dispose);
  return Uri.parse(parameter.value);
}

return RemoteParameter<String>(
value: getString(key),
onConfigUpdated: onConfigUpdated.where(
(config) => config.updatedKeys.contains(key),
),
activateAndRefetch: () async {
await fetchAndActivate();
return getString(key);
},
subscription: filteredOnConfigUpdated(key).listen((event) async {
await activate();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://firebase.google.com/docs/remote-config/get-started?platform=flutter#add-real-time-listener
The official documentation stated "Automatically fetch new parameter values" and only activate() was mentioned, so we modified it to match the documentation.

onConfigUpdated(getString(key));
}),
);
}

/// Returns a [RemoteParameter] of type [int].
RemoteParameter<int> getIntParameter(String key) {
RemoteParameter<int> getIntParameter(
String key, {
required ValueChanged<int> onConfigUpdated,
}) {
return RemoteParameter<int>(
value: getInt(key),
onConfigUpdated: onConfigUpdated.where(
(config) => config.updatedKeys.contains(key),
),
activateAndRefetch: () async {
await fetchAndActivate();
return getInt(key);
},
subscription: filteredOnConfigUpdated(key).listen((event) async {
await activate();
onConfigUpdated(getInt(key));
}),
);
}

/// Returns a [RemoteParameter] of type [double].
RemoteParameter<double> getDoubleParameter(String key) {
RemoteParameter<double> getDoubleParameter(
String key, {
required ValueChanged<double> onConfigUpdated,
}) {
return RemoteParameter<double>(
value: getDouble(key),
onConfigUpdated: onConfigUpdated.where(
(config) => config.updatedKeys.contains(key),
),
activateAndRefetch: () async {
await fetchAndActivate();
return getDouble(key);
},
subscription: filteredOnConfigUpdated(key).listen((event) async {
await activate();
onConfigUpdated(getDouble(key));
}),
);
}

/// Returns a [RemoteParameter] of type [bool].
RemoteParameter<bool> getBoolParameter(String key) {
RemoteParameter<bool> getBoolParameter(
String key, {
required ValueChanged<bool> onConfigUpdated,
}) {
return RemoteParameter<bool>(
value: getBool(key),
onConfigUpdated: onConfigUpdated.where(
(config) => config.updatedKeys.contains(key),
),
activateAndRefetch: () async {
await fetchAndActivate();
return getBool(key);
},
subscription: filteredOnConfigUpdated(key).listen((event) async {
await activate();
onConfigUpdated(getBool(key));
}),
);
}

/// Returns a [RemoteParameter] of type [Map].
RemoteParameter<Map<String, Object?>> getJsonParameter(String key) {
RemoteParameter<Map<String, Object?>> getJsonParameter(
String key, {
required void Function(Map<String, Object?> value) onConfigUpdated,
naipaka marked this conversation as resolved.
Show resolved Hide resolved
}) {
return RemoteParameter<Map<String, Object?>>(
value: getJson(key),
onConfigUpdated: onConfigUpdated.where(
(config) => config.updatedKeys.contains(key),
),
activateAndRefetch: () async {
await fetchAndActivate();
return getJson(key);
},
subscription: filteredOnConfigUpdated(key).listen((event) async {
await activate();
onConfigUpdated(getJson(key));
}),
);
}

/// Returns a [RemoteParameter] of type [List] of [Map].
RemoteParameter<List<Map<String, Object?>>> getListJsonParameter(String key) {
RemoteParameter<List<Map<String, Object?>>> getListJsonParameter(
String key, {
required ValueChanged<List<Map<String, Object?>>> onConfigUpdated,
}) {
return RemoteParameter<List<Map<String, Object?>>>(
value: getListJson(key),
onConfigUpdated: onConfigUpdated.where(
(config) => config.updatedKeys.contains(key),
),
activateAndRefetch: () async {
await fetchAndActivate();
return getListJson(key);
},
subscription: filteredOnConfigUpdated(key).listen((event) async {
await activate();
onConfigUpdated(getListJson(key));
}),
);
}

/// Returns a [RemoteParameter] of type [T].
RemoteParameter<T> getDataParameter<T extends Object>({
required String key,
RemoteParameter<T> getDataParameter<T extends Object>(
String key, {
required T Function(Map<String, Object?>) fromJson,
required ValueChanged<T> onConfigUpdated,
}) {
return RemoteParameter<T>(
value: getData<T>(key: key, fromJson: fromJson),
onConfigUpdated: onConfigUpdated.where(
(config) => config.updatedKeys.contains(key),
),
activateAndRefetch: () async {
await fetchAndActivate();
return getData<T>(key: key, fromJson: fromJson);
},
subscription: filteredOnConfigUpdated(key).listen((event) async {
await activate();
onConfigUpdated(getData(key: key, fromJson: fromJson));
}),
);
}
}
Loading