Make Http request retry limit a read from flags.
Bug: 322865342
Test: atest FederatedComputeServicesTests[com.google.android.ondevicepersonalization.apex]
Change-Id: I56755168b8715dc1284244758b4f3ab3120daa51
diff --git a/federatedcompute/src/com/android/federatedcompute/services/common/Flags.java b/federatedcompute/src/com/android/federatedcompute/services/common/Flags.java
index b7bcfce..e42ebf0 100644
--- a/federatedcompute/src/com/android/federatedcompute/services/common/Flags.java
+++ b/federatedcompute/src/com/android/federatedcompute/services/common/Flags.java
@@ -178,4 +178,10 @@
default Long getAuthorizationTokenDeletionPeriodSeconds() {
return ODP_AUTHORIZATION_TOKEN_DELETION_PERIOD_SECONDs;
}
+
+ int HTTP_REQUEST_RETRY_LIMIT = 3;
+
+ default int getHttpRequestRetryLimit() {
+ return HTTP_REQUEST_RETRY_LIMIT;
+ }
}
diff --git a/federatedcompute/src/com/android/federatedcompute/services/common/PhFlags.java b/federatedcompute/src/com/android/federatedcompute/services/common/PhFlags.java
index d0e8cc1..185948d 100644
--- a/federatedcompute/src/com/android/federatedcompute/services/common/PhFlags.java
+++ b/federatedcompute/src/com/android/federatedcompute/services/common/PhFlags.java
@@ -41,6 +41,9 @@
static final String ENABLE_BACKGROUND_ENCRYPTION_KEY_FETCH =
"enable_background_encryption_key_fetch";
+
+ static final String HTTP_REQUEST_RETRY_LIMIT_CONFIG_NAME =
+ "http_request_retry_limit";
private static final PhFlags sSingleton = new PhFlags();
/** Returns the singleton instance of the PhFlags. */
@@ -82,4 +85,12 @@
/* name= */ ENABLE_BACKGROUND_ENCRYPTION_KEY_FETCH,
/* defaultValue= */ USE_BACKGROUND_ENCRYPTION_KEY_FETCH);
}
+
+ @Override
+ public int getHttpRequestRetryLimit() {
+ return DeviceConfig.getInt(
+ /* namespace= */ NAMESPACE_ON_DEVICE_PERSONALIZATION,
+ /* name= */ HTTP_REQUEST_RETRY_LIMIT_CONFIG_NAME,
+ /* defaultValue= */ HTTP_REQUEST_RETRY_LIMIT);
+ }
}
diff --git a/federatedcompute/src/com/android/federatedcompute/services/http/HttpClient.java b/federatedcompute/src/com/android/federatedcompute/services/http/HttpClient.java
index acbd8c0..5bfd162 100644
--- a/federatedcompute/src/com/android/federatedcompute/services/http/HttpClient.java
+++ b/federatedcompute/src/com/android/federatedcompute/services/http/HttpClient.java
@@ -23,6 +23,8 @@
import android.annotation.Nullable;
import com.android.federatedcompute.internal.util.LogUtil;
+import com.android.federatedcompute.services.common.Flags;
+import com.android.federatedcompute.services.common.PhFlags;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.util.concurrent.Futures;
@@ -45,11 +47,13 @@
*/
public class HttpClient {
private static final String TAG = HttpClient.class.getSimpleName();
- private static final int RETRY_LIMIT = 3;
private static final int NETWORK_CONNECT_TIMEOUT_MS = (int) TimeUnit.SECONDS.toMillis(5);
private static final int NETWORK_READ_TIMEOUT_MS = (int) TimeUnit.SECONDS.toMillis(30);
+ private final Flags mFlags;
- public HttpClient() {}
+ public HttpClient() {
+ mFlags = PhFlags.getInstance();
+ }
@NonNull
@VisibleForTesting
@@ -81,7 +85,8 @@
throws IOException {
int count = 0;
FederatedComputeHttpResponse response = null;
- while (count < RETRY_LIMIT) {
+ int retryLimit = mFlags.getHttpRequestRetryLimit();
+ while (count < retryLimit) {
try {
response = performRequest(request);
if (HTTP_OK_STATUS.contains(response.getStatusCode())) {
@@ -90,7 +95,7 @@
// we want to continue retry in case it is IO exception.
} catch (IOException e) {
// propagate IO exception after RETRY_LIMIT times attempt.
- if (count >= RETRY_LIMIT - 1) {
+ if (count >= retryLimit - 1) {
throw e;
}
} finally {
diff --git a/tests/federatedcomputetests/src/com/android/federatedcompute/services/common/PhFlagsTest.java b/tests/federatedcomputetests/src/com/android/federatedcompute/services/common/PhFlagsTest.java
index 9765124..92900d8 100644
--- a/tests/federatedcomputetests/src/com/android/federatedcompute/services/common/PhFlagsTest.java
+++ b/tests/federatedcomputetests/src/com/android/federatedcompute/services/common/PhFlagsTest.java
@@ -17,15 +17,18 @@
package com.android.federatedcompute.services.common;
import static com.android.federatedcompute.services.common.Flags.FEDERATED_COMPUTE_GLOBAL_KILL_SWITCH;
+import static com.android.federatedcompute.services.common.Flags.HTTP_REQUEST_RETRY_LIMIT;
import static com.android.federatedcompute.services.common.Flags.USE_BACKGROUND_ENCRYPTION_KEY_FETCH;
import static com.android.federatedcompute.services.common.PhFlags.ENABLE_BACKGROUND_ENCRYPTION_KEY_FETCH;
import static com.android.federatedcompute.services.common.PhFlags.FEDERATED_COMPUTATION_ENCRYPTION_KEY_DOWNLOAD_URL;
+import static com.android.federatedcompute.services.common.PhFlags.HTTP_REQUEST_RETRY_LIMIT_CONFIG_NAME;
import static com.android.federatedcompute.services.common.PhFlags.KEY_FEDERATED_COMPUTE_KILL_SWITCH;
import static com.google.common.truth.Truth.assertThat;
import android.provider.DeviceConfig;
+import org.junit.AfterClass;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -40,6 +43,24 @@
PhFlagsTestUtil.setUpDeviceConfigPermissions();
}
+ /**
+ * Roll flags back to their default values.
+ */
+ @AfterClass
+ public static void tearDown() {
+ // roll back to default value.
+ DeviceConfig.setProperty(
+ DeviceConfig.NAMESPACE_ON_DEVICE_PERSONALIZATION,
+ HTTP_REQUEST_RETRY_LIMIT_CONFIG_NAME,
+ Integer.toString(HTTP_REQUEST_RETRY_LIMIT),
+ /* makeDefault= */ false);
+ DeviceConfig.setProperty(
+ DeviceConfig.NAMESPACE_ON_DEVICE_PERSONALIZATION,
+ ENABLE_BACKGROUND_ENCRYPTION_KEY_FETCH,
+ Boolean.toString(USE_BACKGROUND_ENCRYPTION_KEY_FETCH),
+ /* makeDefault= */ false);
+ }
+
@Test
public void testGetGlobalKillSwitch() {
// Without any overriding, the value is the hard coded constant.
@@ -52,7 +73,7 @@
.isEqualTo(FEDERATED_COMPUTE_GLOBAL_KILL_SWITCH);
// Now overriding with the value from PH.
- final boolean phOverridingValue = true;
+ final boolean phOverridingValue = !FEDERATED_COMPUTE_GLOBAL_KILL_SWITCH;
DeviceConfig.setProperty(
DeviceConfig.NAMESPACE_ON_DEVICE_PERSONALIZATION,
KEY_FEDERATED_COMPUTE_KILL_SWITCH,
@@ -89,7 +110,7 @@
.isEqualTo(USE_BACKGROUND_ENCRYPTION_KEY_FETCH);
// Now overriding the value from PH.
- boolean overrideEnableBackgroundKeyFetch = false;
+ boolean overrideEnableBackgroundKeyFetch = !USE_BACKGROUND_ENCRYPTION_KEY_FETCH;
DeviceConfig.setProperty(
DeviceConfig.NAMESPACE_ON_DEVICE_PERSONALIZATION,
ENABLE_BACKGROUND_ENCRYPTION_KEY_FETCH,
@@ -101,4 +122,28 @@
.isEqualTo(overrideEnableBackgroundKeyFetch);
}
+
+ @Test
+ public void testGetHttpRequestRetryLimit() {
+ // Without Overriding
+ DeviceConfig.setProperty(
+ DeviceConfig.NAMESPACE_ON_DEVICE_PERSONALIZATION,
+ HTTP_REQUEST_RETRY_LIMIT_CONFIG_NAME,
+ Integer.toString(HTTP_REQUEST_RETRY_LIMIT),
+ /* makeDefault= */ false);
+ assertThat(FlagsFactory.getFlags().getHttpRequestRetryLimit())
+ .isEqualTo(HTTP_REQUEST_RETRY_LIMIT);
+
+ // Now overriding the value from PH.
+ int overrideHttpRequestRetryLimit = 4;
+ DeviceConfig.setProperty(
+ DeviceConfig.NAMESPACE_ON_DEVICE_PERSONALIZATION,
+ HTTP_REQUEST_RETRY_LIMIT_CONFIG_NAME,
+ Integer.toString(overrideHttpRequestRetryLimit),
+ /* makeDefault= */ false);
+
+ Flags phFlags = FlagsFactory.getFlags();
+ assertThat(phFlags.getHttpRequestRetryLimit())
+ .isEqualTo(overrideHttpRequestRetryLimit);
+ }
}
