Merged

fix(app-check, web): fixed broken onTokenChanged and ensured it is properly cleaned up. Streams are also cleaned up on "hot restart" #12933

Show file tree
Hide file tree
Changes from all commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
23a4097
chore: reinsert debug token initialiser
russellwheatleyJun 10, 2024
b485792
fix(app-check): broken stream handling
russellwheatleyJun 10, 2024
8dd9ff5
fix: app check stream clean up on hot restart
russellwheatleyJun 10, 2024
d2f8e4e
chore: format
russellwheatleyJun 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Failed to load files.
Original file line numberDiff line numberDiff line change
Expand Up@@ -33,6 +33,9 @@
<link rel="manifest" href="manifest.json">
</head>
<body>
<script>
self.FIREBASE_APPCHECK_DEBUG_TOKEN = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it supposed to stay?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, just allows us to use debug token to test when we run the example app 👍

</script>
<script src="flutter_bootstrap.js" async></script>
</body>
</html>
Original file line numberDiff line numberDiff line change
Expand Up@@ -19,7 +19,6 @@ import 'src/interop/app_check.dart' as app_check_interop;
class FirebaseAppCheckWeb extends FirebaseAppCheckPlatform {
static const recaptchaTypeV3 = 'recaptcha-v3';
static const recaptchaTypeEnterprise = 'enterprise';

static Map<String, StreamController<String?>> _tokenChangesListeners = {};

/// Stub initializer to allow the [registerWith] to create an instance without
Expand DownExpand Up@@ -121,17 +120,25 @@ class FirebaseAppCheckWeb extends FirebaseAppCheckPlatform {
return convertWebExceptions<Future<void>>(() async {
_webAppCheck ??= app_check_interop.getAppCheckInstance(
core_interop.app(app.name), webProvider);
if (_tokenChangesListeners[app.name] == null) {
_tokenChangesListeners[app.name] =
StreamController<String?>.broadcast();

_delegate!.onTokenChanged().map((event) {
_tokenChangesListeners[app.name]!.add(event.token.toDart);
});
}
_initialiseStreamController();
});
}

void _initialiseStreamController() {
if (_tokenChangesListeners[app.name] == null) {
_tokenChangesListeners[app.name] = StreamController<String?>.broadcast(
onCancel: () {
_tokenChangesListeners[app.name]!.close();
_tokenChangesListeners.remove(app.name);
_delegate!.idTokenChangedController?.close();
},
);
_delegate!.onTokenChanged(app.name).listen((event) {
_tokenChangesListeners[app.name]!.add(event.token.toDart);
});
}
}

@override
Future<String?> getToken(bool forceRefresh) async {
return convertWebExceptions<Future<String?>>(() async {
Expand DownExpand Up@@ -162,6 +169,9 @@ class FirebaseAppCheckWeb extends FirebaseAppCheckPlatform {

@override
Stream<String?> get onTokenChange {
return _tokenChangesListeners[app.name]!.stream;
_initialiseStreamController();
return convertWebExceptions(
() => _tokenChangesListeners[app.name]!.stream,
);
}
}
Original file line numberDiff line numberDiff line change
Expand Up@@ -69,11 +69,18 @@ class AppCheck extends JsObjectWrapper<app_check_interop.AppCheckJsImpl> {

JSFunction? _idTokenChangedUnsubscribe;

StreamController<app_check_interop.AppCheckTokenResult>?
get idTokenChangedController => _idTokenChangedController;

StreamController<app_check_interop.AppCheckTokenResult>?
// ignore: close_sinks
_idTokenChangedController;

Stream<app_check_interop.AppCheckTokenResult> onTokenChanged() {
String _appCheckWindowsKey(String appName) =>
'flutterfire-${appName}_onTokenChanged';
Stream<app_check_interop.AppCheckTokenResult> onTokenChanged(String appName) {
final appCheckWindowsKey = _appCheckWindowsKey(appName);
unsubscribeWindowsListener(appCheckWindowsKey);
if (_idTokenChangedController == null) {
final nextWrapper = ((app_check_interop.AppCheckTokenResult result) {
_idTokenChangedController!.add(result);
Expand All@@ -83,17 +90,19 @@ class AppCheck extends JsObjectWrapper<app_check_interop.AppCheckJsImpl> {
((JSError e) => _idTokenChangedController!.addError(e)).toJS;

void startListen() {
assert(_idTokenChangedUnsubscribe == null);
_idTokenChangedUnsubscribe = app_check_interop.onTokenChanged(
jsObject,
nextWrapper,
errorWrapper,
);
setWindowsListener(appCheckWindowsKey, _idTokenChangedUnsubscribe!);
}

void stopListen() {
_idTokenChangedUnsubscribe?.callAsFunction();
_idTokenChangedUnsubscribe = null;
_idTokenChangedController = null;
removeWindowsListener(appCheckWindowsKey);
}

_idTokenChangedController =
Expand Down
Loading