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.

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

unsubscribe listenToChanges firestore #1508

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 19 additions & 4 deletions modules/ensemble/lib/action/invoke_api_action.dart
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class InvokeAPIAction extends EnsembleAction {
}

class InvokeAPIController {
Future<Response> executeWithContext(
Future<Response?> executeWithContext(
BuildContext context, InvokeAPIAction action,
{Map<String, dynamic>? additionalInputs}) {
ScopeManager? foundScopeManager =
Expand All @@ -90,9 +90,10 @@ class InvokeAPIController {
return APIProviders.of(context).getProvider(provider);
}

Future<Response> execute(InvokeAPIAction action, BuildContext context,
Future<Response?> execute(InvokeAPIAction action, BuildContext context,
ScopeManager scopeManager, Map<String, YamlMap>? apiMap) async {
YamlMap? apiDefinition = apiMap?[action.apiName];

if (apiDefinition != null) {
ScopeManager apiScopeManager =
scopeManager.newCreateChildScope(ephemeral: true);
Expand Down Expand Up @@ -144,13 +145,27 @@ class InvokeAPIController {
}

APIProvider apiProvider = getAPIProvider(context, apiDefinition);
var isChangeListener = apiDefinition['listenForChanges'];

if (isChangeListener is! bool) {
isChangeListener = apiScopeManager.dataContext
.getContextById(apiDefinition['listenForChanges']);

if (!isChangeListener) {
if (apiProvider is LiveAPIProvider) {
await (apiProvider as LiveAPIProvider)
.unSubscribeToApi(action.apiName);
Copy link
Collaborator

Choose a reason for hiding this comment

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

we need to consider the API's with the same name but different IDs. The use case there is calling the same API with different inputs and using the API's id to bind it to different widgets. This code will cancel subscriptions for any existing APIs with the same name but different ids

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree. I'll implement once I prioritise it.

}
return null;
}
}

if (AppConfig(context, apiScopeManager.dataContext.getAppId())
.isMockResponse() &&
apiDefinition['mockResponse'] != null) {
response = await apiProvider.invokeMockAPI(
apiScopeManager.dataContext, apiDefinition['mockResponse']);
} else if (apiDefinition['listenForChanges'] == true &&
apiProvider is LiveAPIProvider) {
} else if (isChangeListener == true && apiProvider is LiveAPIProvider) {
response = await (apiProvider as LiveAPIProvider).subscribeToApi(
context,
apiDefinition,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ abstract class APIProvider {
mixin LiveAPIProvider {
Future<Response> subscribeToApi(BuildContext context, YamlMap api,
DataContext eContext, String apiName, ResponseListener listener);
Future<void> unSubscribeToApi(String apiName);
}

class APIProviders extends InheritedWidget {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,13 @@ import 'package:firebase_core/firebase_core.dart';
import 'package:flutter/foundation.dart';

class FirestoreAPIProvider extends APIProvider with LiveAPIProvider {
FirestoreAPIProvider._();
static final FirestoreAPIProvider _instance = FirestoreAPIProvider._();
factory FirestoreAPIProvider() => _instance;

FirebaseApp? _app;
FirebaseFirestore get firestore => FirebaseFirestore.instanceFor(app: _app!);
List<StreamSubscription> _subscriptions = [];
Map<String, StreamSubscription> _subscriptions = {};
late FirestoreApp firestoreApp;

@override
Expand Down Expand Up @@ -237,6 +241,12 @@ class FirestoreAPIProvider extends APIProvider with LiveAPIProvider {
throw UnimplementedError();
}

@override
Future<void> unSubscribeToApi(String apiName) async {
if (!_subscriptions.containsKey(apiName)) return;
await _subscriptions[apiName]?.cancel();
}

@override
Future<FirestoreResponse> subscribeToApi(BuildContext context, YamlMap apiMap,
DataContext eContext, String apiName, ResponseListener listener) async {
Expand All @@ -257,20 +267,25 @@ class FirestoreAPIProvider extends APIProvider with LiveAPIProvider {

if (isDocumentPath) {
DocumentReference docRef = firestore.doc(path);
_subscriptions.add(docRef.snapshots().listen((DocumentSnapshot snapshot) {
listener.call(getOKResponse(apiName, snapshot));
}));

_subscriptions.addAll({
apiName: docRef.snapshots().listen((DocumentSnapshot snapshot) {
listener.call(getOKResponse(apiName, snapshot));
})
});
} else {
Query query = firestoreApp.getQuery(api);
_subscriptions.add(query.snapshots().listen((QuerySnapshot snapshot) {
listener.call(getOKResponse(apiName, snapshot));
}));
_subscriptions.addAll({
apiName: query.snapshots().listen((QuerySnapshot snapshot) {
listener.call(getOKResponse(apiName, snapshot));
})
});
}
Map<String, dynamic> body = {
'message': 'Subscribed to API',
'documents': []
};
;

return FirestoreResponse(
apiState: APIState.success,
body: body,
Expand All @@ -283,11 +298,11 @@ class FirestoreAPIProvider extends APIProvider with LiveAPIProvider {
dispose() {
try {
if (_subscriptions.isNotEmpty) {
for (var subscription in _subscriptions) {
for (var subscription in _subscriptions.values) {
subscription.cancel();
}
}
_subscriptions = [];
_subscriptions = {};
} catch (e) {
print('Error disposing FirestoreAPIProvider: $e');
}
Expand Down
Loading