-
Notifications
You must be signed in to change notification settings - Fork 660
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
Espresso idling resource for Apollo GraphQL client #469
Changes from 1 commit
7a1664d
3ad4610
1210f32
0dda9b8
51817a7
76780b3
0af44d1
989e102
b68e8e5
ff6fa0d
7158335
873f393
5525529
43a90b6
53801bf
4483920
190ae11
664b033
41dcb1d
72484ae
cbf6574
de9a829
1cc6187
07d4b91
5d23a9f
69a28d5
d934cc0
8d1a6d6
7610a67
c75d4c4
79d2f99
f5ba765
85581e0
7471952
f3f1e48
fa598f1
0d495e1
8133815
1860193
f47672b
c8102ba
dd3cb51
95c9383
f9d36a8
ac1e4a8
516615b
4fab454
57c401d
c4e5ab5
3dad93e
9820c66
2096424
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
Not exposing apolloTracker to the clients. Instead they have to call apolloClient's getRunningCount method and setIdleCallback method which then delegates these calls to the apolloTracker
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -74,7 +74,7 @@ public static Builder builder() { | |
private final HttpCacheControl defaultHttpCacheControl; | ||
private final CacheControl defaultCacheControl; | ||
private final ApolloLogger logger; | ||
private final com.apollographql.apollo.internal.ApolloCallTracker callTracker = new com.apollographql.apollo.internal.ApolloCallTracker(); | ||
private final ApolloCallTracker callTracker = new ApolloCallTracker(); | ||
private final List<ApolloInterceptor> applicationInterceptors; | ||
|
||
private ApolloClient(Builder builder) { | ||
|
@@ -142,8 +142,12 @@ public ApolloStore apolloStore() { | |
return apolloStore; | ||
} | ||
|
||
public com.apollographql.apollo.internal.ApolloCallTracker apolloCallTracker() { | ||
return callTracker; | ||
public void setIdleCallback(Runnable idleCallback) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove prefix There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And should it be part of Builder? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel it shouldn't be a part of the builder because it will force the clients to supply the callback at the time of creation of the Keeping it separate from the builder lets the clients create |
||
callTracker.setIdleCallback(idleCallback); | ||
} | ||
|
||
public int getRunningCallsCount() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. drop prefix |
||
return callTracker.getRunningCallsCount(); | ||
} | ||
|
||
Response cachedHttpResponse(String cacheKey) throws IOException { | ||
|
@@ -308,8 +312,8 @@ public Builder logger(@Nullable Logger logger) { | |
* after the response source is selected (either the server, cache or both). This method can be called multiple | ||
* times for adding multiple application interceptors. </p> | ||
* | ||
* <p>Note: Interceptors will be called <b>in the order in which they are added to the list of interceptors</b> | ||
* and if any of the interceptors tries to short circuit the responses, then subsequent interceptors <b>won't</b> be | ||
* <p>Note: Interceptors will be called <b>in the order in which they are added to the list of interceptors</b> and | ||
* if any of the interceptors tries to short circuit the responses, then subsequent interceptors <b>won't</b> be | ||
* called.</p> | ||
* | ||
* @param interceptor Application level interceptor to add | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we need to make it lazy, so if I don't need to tracker we should avoid overhead of
ApolloCallTracker
(it uses synchronization blocks a lot)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am wondering how we are going to lazily create it. If we had a builder method to supply the
IdleCallback
then it would have been possible, but I have already expressed my concern over why we shouldn't take that route.Or maybe if we are thinking of creating the ApolloTracker when someone sets the idleCallback, then I am afraid that wouldn't give us an exact count of running calls e.g. if an async call is made and then an idleCallback is set, then the apolloTracker would give us 0 running calls even though we have a running call.
Do you have anything else in your mind?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I meant is if user doesn't care about keep tracking of calls, then why should we create it at all and have some synchronization overhead.
But at the same time for bigger picture we probably need it.
Then I would call it not a tracker but
ActiveCallRegistry
or smth like this, and it can be used not only for tracking how many calls we are running right now but potentially for batching requests. In this case we need to include as wellApolloWatcher
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would probably update this class later to enable this feature #452
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright.