Skip to content

Inject executor service for Realtime components. #4375

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

Merged
merged 1 commit into from
Dec 1, 2022
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import java.util.concurrent.Executor;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.atomic.AtomicReference;

/**
Expand Down Expand Up @@ -84,7 +85,9 @@ public class RemoteConfigComponent {
new HashMap<>();

private final Context context;
// TODO: Consolidate executors.
private final ExecutorService executorService;
private final ScheduledExecutorService scheduledExecutorService;
private final FirebaseApp firebaseApp;
private final FirebaseInstallationsApi firebaseInstallations;
private final FirebaseABTesting firebaseAbt;
Expand All @@ -105,6 +108,7 @@ public class RemoteConfigComponent {
this(
context,
Executors.newCachedThreadPool(),
Executors.newSingleThreadScheduledExecutor(),
firebaseApp,
firebaseInstallations,
firebaseAbt,
Expand All @@ -117,6 +121,7 @@ public class RemoteConfigComponent {
protected RemoteConfigComponent(
Context context,
ExecutorService executorService,
ScheduledExecutorService scheduledExecutorService,
FirebaseApp firebaseApp,
FirebaseInstallationsApi firebaseInstallations,
FirebaseABTesting firebaseAbt,
Expand All @@ -128,6 +133,7 @@ protected RemoteConfigComponent(
this.firebaseInstallations = firebaseInstallations;
this.firebaseAbt = firebaseAbt;
this.analyticsConnector = analyticsConnector;
this.scheduledExecutorService = scheduledExecutorService;

this.appId = firebaseApp.getOptions().getApplicationId();
GlobalBackgroundListener.ensureBackgroundListenerIsRegistered(context);
Expand Down Expand Up @@ -272,7 +278,8 @@ synchronized ConfigRealtimeHandler getRealtime(
configFetchHandler,
context,
namespace,
executorService);
executorService,
scheduledExecutorService);
}

private ConfigGetParameterHandler getGetHandler(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import java.net.HttpURLConnection;
import java.util.Random;
import java.util.Set;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
import org.json.JSONException;
Expand All @@ -58,12 +57,13 @@ public ConfigAutoFetch(
HttpURLConnection httpURLConnection,
ConfigFetchHandler configFetchHandler,
Set<ConfigUpdateListener> eventListeners,
ConfigUpdateListener retryCallback) {
ConfigUpdateListener retryCallback,
ScheduledExecutorService scheduledExecutorService) {
this.httpURLConnection = httpURLConnection;
this.configFetchHandler = configFetchHandler;
this.eventListeners = eventListeners;
this.retryCallback = retryCallback;
this.scheduledExecutorService = Executors.newSingleThreadScheduledExecutor();
this.scheduledExecutorService = scheduledExecutorService;
this.random = new Random();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Future;
import java.util.concurrent.FutureTask;
import java.util.concurrent.ScheduledExecutorService;

public class ConfigRealtimeHandler {

Expand All @@ -42,14 +43,16 @@ public class ConfigRealtimeHandler {
private final Context context;
private final String namespace;
private final ExecutorService executorService;
private final ScheduledExecutorService scheduledExecutorService;

public ConfigRealtimeHandler(
FirebaseApp firebaseApp,
FirebaseInstallationsApi firebaseInstallations,
ConfigFetchHandler configFetchHandler,
Context context,
String namespace,
ExecutorService executorService) {
ExecutorService executorService,
ScheduledExecutorService scheduledExecutorService) {

this.listeners = new LinkedHashSet<>();
this.realtimeHttpClientTask = null;
Expand All @@ -60,6 +63,7 @@ public ConfigRealtimeHandler(
this.context = context;
this.namespace = namespace;
this.executorService = executorService;
this.scheduledExecutorService = scheduledExecutorService;
}

private synchronized boolean canCreateRealtimeHttpClientTask() {
Expand Down Expand Up @@ -92,7 +96,8 @@ private synchronized void beginRealtime() {
configFetchHandler,
context,
namespace,
listeners);
listeners,
scheduledExecutorService);
this.realtimeHttpClientTask =
this.executorService.submit(
new RealtimeHttpClientFutureTask(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
import java.util.Map;
import java.util.Random;
import java.util.Set;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
import java.util.regex.Matcher;
Expand Down Expand Up @@ -93,11 +92,12 @@ public ConfigRealtimeHttpClient(
ConfigFetchHandler configFetchHandler,
Context context,
String namespace,
Set<ConfigUpdateListener> listeners) {
Set<ConfigUpdateListener> listeners,
ScheduledExecutorService scheduledExecutorService) {

this.listeners = listeners;
this.httpURLConnection = null;
this.scheduledExecutorService = Executors.newSingleThreadScheduledExecutor();
this.scheduledExecutorService = scheduledExecutorService;

// Retry parameters
this.random = new Random();
Expand Down Expand Up @@ -303,7 +303,8 @@ public void onError(@NonNull FirebaseRemoteConfigException error) {
}
};

return new ConfigAutoFetch(httpURLConnection, configFetchHandler, listeners, retryCallback);
return new ConfigAutoFetch(
httpURLConnection, configFetchHandler, listeners, retryCallback, scheduledExecutorService);
}

// HTTP status code that the Realtime client should retry on.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@
import java.util.Map;
import java.util.Set;
import java.util.concurrent.Executor;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import org.json.JSONArray;
import org.json.JSONException;
import org.json.JSONObject;
Expand Down Expand Up @@ -180,6 +182,9 @@ public final class FirebaseRemoteConfigTest {

private FetchResponse firstFetchedContainerResponse;

private final ScheduledExecutorService scheduledExecutorService =
Executors.newSingleThreadScheduledExecutor();

@Before
public void setUp() throws Exception {
DEFAULTS_MAP.put("first_default_key", "first_default_value");
Expand Down Expand Up @@ -304,15 +309,21 @@ public void onError(@NonNull FirebaseRemoteConfigException error) {

listeners.add(listener);
configAutoFetch =
new ConfigAutoFetch(mockHttpURLConnection, mockFetchHandler, listeners, mockRetryListener);
new ConfigAutoFetch(
mockHttpURLConnection,
mockFetchHandler,
listeners,
mockRetryListener,
scheduledExecutorService);
configRealtimeHttpClient =
new ConfigRealtimeHttpClient(
firebaseApp,
mockFirebaseInstallations,
mockFetchHandler,
context,
"firebase",
listeners);
listeners,
scheduledExecutorService);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@
import com.google.firebase.remoteconfig.internal.ConfigGetParameterHandler;
import com.google.firebase.remoteconfig.internal.ConfigMetadataClient;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -76,6 +78,7 @@ public class RemoteConfigComponentTest {

private Context context;
private ExecutorService directExecutor;
private ScheduledExecutorService scheduledExecutorService;
private FirebaseApp defaultApp;

@Before
Expand All @@ -84,6 +87,7 @@ public void setUp() {

context = ApplicationProvider.getApplicationContext();
directExecutor = MoreExecutors.newDirectExecutorService();
scheduledExecutorService = Executors.newSingleThreadScheduledExecutor();

defaultApp = initializeFirebaseApp(context);

Expand Down Expand Up @@ -172,6 +176,7 @@ private RemoteConfigComponent getNewFrcComponent() {
return new RemoteConfigComponent(
context,
directExecutor,
scheduledExecutorService,
mockFirebaseApp,
mockFirebaseInstallations,
mockFirebaseAbt,
Expand All @@ -183,6 +188,7 @@ private RemoteConfigComponent getNewFrcComponentWithoutLoadingDefault() {
return new RemoteConfigComponent(
context,
directExecutor,
scheduledExecutorService,
mockFirebaseApp,
mockFirebaseInstallations,
mockFirebaseAbt,
Expand Down