graphql-flutter
graphql-flutter copied to clipboard
fix(graphql): deduplicate pollers
Fixes / Enhancements
Fixed Issue
Calling stopPolling, followed quickly by startPolling, does not remove the first poller and results in duplicate pollers.
Reproducing issue
Create an observable query and call
void startDupPolling(Duration duration) {
_observableQuery.startPolling(duration);
_observableQuery.startPolling(duration); // Internally calls `stopPolling` if poller is running
}
Or
void startDupPolling(Duration duration) {
_observableQuery.startPolling(duration);
_observableQuery.stopPolling();
_observableQuery.startPolling(duration);
}
Starts two pollers instead of one.
Explanation
/// Removes the [ObservableQuery] from one of the registered queries.
/// The fetchQueriesOnInterval will then take care of not firing it anymore.
void stopPollingQuery(String queryId) {
registeredQueries.remove(queryId);
}
stopPolling is expecting fetchQueriesOnInterval to remove the stopped poll. This only happens if registeredQueries[queryId] == null is true when fetchQueriesOnInterval is run. Calling startPolling again quickly, adds a new entry to registeredQueries[queryId] before fetchQueriesOnInterval, thus leading to duplicate entries in intervalQueries. Since registeredQueries[queryId] is true, all entries for the queryId in intervalQueries get fired, instead of the stopped ones being removed.
The Fix
By making intervalQueries store a set, we prevent duplicate entries from being added for the same duration. Thus the value of registeredQueries[queryId] applies to the only value for that queryId in the set.
Codecov Report
Merging #1240 (0990ed7) into main (312dc6c) will increase coverage by
3.68%. The diff coverage is100.00%.
:exclamation: Current head 0990ed7 differs from pull request most recent head 3cc0942. Consider uploading reports for the commit 3cc0942 to get more accurate results
@@ Coverage Diff @@
## main #1240 +/- ##
==========================================
+ Coverage 59.29% 62.98% +3.68%
==========================================
Files 41 41
Lines 1683 1683
==========================================
+ Hits 998 1060 +62
+ Misses 685 623 -62
| Impacted Files | Coverage Δ | |
|---|---|---|
| packages/graphql/lib/src/cache/cache.dart | 78.57% <ø> (ø) |
|
| packages/graphql/lib/src/scheduler/scheduler.dart | 100.00% <100.00%> (+95.83%) |
:arrow_up: |
| packages/graphql/lib/src/core/query_manager.dart | 77.55% <0.00%> (+1.36%) |
:arrow_up: |
| packages/graphql/lib/src/core/query_options.dart | 57.48% <0.00%> (+8.38%) |
:arrow_up: |
| ...ackages/graphql/lib/src/core/observable_query.dart | 55.30% <0.00%> (+17.42%) |
:arrow_up: |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
Thanks! I will put on my todo list, overall looks good but I need to clone and see in deep these changes!
Would love to get this merged in so we can get back on the latest version of the package. Let me know if there's anything I can help with.
Thanks for this fix!