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.

Sign up for GitHub

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

Update fetch to utilize latest custom signals #6582

Merged
merged 5 commits into from
Dec 10, 2024
Merged

Update fetch to utilize latest custom signals #6582

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cb8d78d
Ensure Fetch Uses Latest Custom Signals
tusharkhandelwal8 Dec 9, 2024
32946b9
Add tests to verify correct behavior.
tusharkhandelwal8 Dec 10, 2024
6738c6d
Verify custom signals in fetch arguments match shared preferences
tusharkhandelwal8 Dec 10, 2024
d887d18
Rename the test
tusharkhandelwal8 Dec 10, 2024
4e44147
Inline the assertion in the test method itself.
tusharkhandelwal8 Dec 10, 2024
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
3 changes: 1 addition & 2 deletions firebase-config/src/main/java/com/google/firebase/remoteconfig/RemoteConfigComponent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,8 +262,7 @@ ConfigFetchHttpClient getFrcBackendApiClient(
apiKey,
namespace,
/* connectTimeoutInSeconds= */ sharedPrefsClient.getFetchTimeoutInSeconds(),
/* readTimeoutInSeconds= */ sharedPrefsClient.getFetchTimeoutInSeconds(),
/* customSignals= */ sharedPrefsClient.getCustomSignals());
/* readTimeoutInSeconds= */ sharedPrefsClient.getFetchTimeoutInSeconds());
}

@VisibleForTesting
Expand Down
3 changes: 2 additions & 1 deletion ...se-config/src/main/java/com/google/firebase/remoteconfig/internal/ConfigFetchHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,8 @@ private FetchResponse fetchFromBackend(
frcSharedPrefs.getLastFetchETag(),
customFetchHeaders,
getFirstOpenTime(),
currentTime);
currentTime,
frcSharedPrefs.getCustomSignals());

if (response.getFetchedConfigs() != null) {
// Set template version in metadata to be saved on disk.
Expand Down
17 changes: 10 additions & 7 deletions ...config/src/main/java/com/google/firebase/remoteconfig/internal/ConfigFetchHttpClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ public class ConfigFetchHttpClient {
private final String apiKey;
private final String projectNumber;
private final String namespace;
Map<String, String> customSignalsMap;
private final long connectTimeoutInSeconds;
private final long readTimeoutInSeconds;

Expand All @@ -108,16 +107,14 @@ public ConfigFetchHttpClient(
String apiKey,
String namespace,
long connectTimeoutInSeconds,
long readTimeoutInSeconds,
Map<String, String> customSignalsMap) {
long readTimeoutInSeconds) {
this.context = context;
this.appId = appId;
this.apiKey = apiKey;
this.projectNumber = extractProjectNumberFromAppId(appId);
this.namespace = namespace;
this.connectTimeoutInSeconds = connectTimeoutInSeconds;
this.readTimeoutInSeconds = readTimeoutInSeconds;
this.customSignalsMap = customSignalsMap;
}

/** Used to verify that the timeout is being set correctly. */
Expand Down Expand Up @@ -187,7 +184,8 @@ FetchResponse fetch(
String lastFetchETag,
Map<String, String> customHeaders,
Long firstOpenTime,
Date currentTime)
Date currentTime,
Map<String, String> customSignalsMap)
throws FirebaseRemoteConfigException {
setUpUrlConnection(urlConnection, lastFetchETag, installationAuthToken, customHeaders);

Expand All @@ -196,7 +194,11 @@ FetchResponse fetch(
try {
byte[] requestBody =
createFetchRequestBody(
installationId, installationAuthToken, analyticsUserProperties, firstOpenTime)
installationId,
installationAuthToken,
analyticsUserProperties,
firstOpenTime,
customSignalsMap)
.toString()
.getBytes("utf-8");
setFetchRequestBody(urlConnection, requestBody);
Expand Down Expand Up @@ -307,7 +309,8 @@ private JSONObject createFetchRequestBody(
String installationId,
String installationAuthToken,
Map<String, String> analyticsUserProperties,
Long firstOpenTime)
Long firstOpenTime,
Map<String, String> customSignalsMap)
throws FirebaseRemoteConfigClientException {
Map<String, Object> requestBodyMap = new HashMap<>();

Expand Down
51 changes: 42 additions & 9 deletions ...onfig/src/test/java/com/google/firebase/remoteconfig/internal/ConfigFetchHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,8 @@ public void fetch_firstFetch_includesInstallationAuthToken() throws Exception {
/* lastFetchETag= */ any(),
/* customHeaders= */ any(),
/* firstOpenTime= */ any(),
/* currentTime= */ any());
/* currentTime= */ any(),
/* customSignals= */ any());
}

@Test
Expand Down Expand Up @@ -401,7 +402,8 @@ public void fetch_gettingFetchCacheFails_doesNotThrowException() throws Exceptio

@Test
public void fetch_fetchBackendCallFails_taskThrowsException() throws Exception {
when(mockBackendFetchApiClient.fetch(any(), any(), any(), any(), any(), any(), any(), any()))
when(mockBackendFetchApiClient.fetch(
any(), any(), any(), any(), any(), any(), any(), any(), any()))
.thenThrow(
new FirebaseRemoteConfigClientException("Fetch failed due to an unexpected error."));

Expand Down Expand Up @@ -766,6 +768,30 @@ public void getInfo_hitsThrottleLimit_throttledStatus() throws Exception {
.isEqualTo(firstFetchedContainer.getFetchTime());
}

@Test
public void fetch_usesLatestCustomSignals() throws Exception {
Map<String, String> customSignals =
ImmutableMap.of(
"subscription", "premium",
"age", "20");
sharedPrefsClient.setCustomSignals(customSignals);
fetchCallToHttpClientUpdatesClockAndReturnsConfig(firstFetchedContainer);
fetchHandler.fetch();

verify(mockBackendFetchApiClient)
.fetch(
any(HttpURLConnection.class),
/* instanceId= */ any(),
/* instanceIdToken= */ any(),
/* analyticsUserProperties= */ any(),
/* lastFetchETag= */ any(),
/* customHeaders= */ any(),
/* firstOpenTime= */ any(),
/* currentTime= */ any(),
/* customSignals= */ eq(sharedPrefsClient.getCustomSignals()));
assertThat(sharedPrefsClient.getCustomSignals()).isEqualTo(customSignals);
}

private ConfigFetchHandler getNewFetchHandler(AnalyticsConnector analyticsConnector) {
ConfigFetchHandler fetchHandler =
spy(
Expand Down Expand Up @@ -811,7 +837,8 @@ private void setBackendResponseConfigsTo(ConfigContainer container) throws Excep
/* lastFetchETag= */ any(),
/* customHeaders= */ any(),
/* firstOpenTime= */ any(),
/* currentTime= */ any());
/* currentTime= */ any(),
/* customSignals= */ any());
}

private void setBackendResponseToNoChange(Date date) throws Exception {
Expand All @@ -823,7 +850,8 @@ private void setBackendResponseToNoChange(Date date) throws Exception {
/* lastFetchETag= */ any(),
/* customHeaders= */ any(),
/* firstOpenTime= */ any(),
/* currentTime= */ any()))
/* currentTime= */ any(),
/* customSignals= */ any()))
.thenReturn(FetchResponse.forBackendHasNoUpdates(date, firstFetchedContainer));
}

Expand All @@ -838,7 +866,8 @@ private void fetchCallToBackendThrowsException(int httpErrorCode) throws Excepti
/* lastFetchETag= */ any(),
/* customHeaders= */ any(),
/* firstOpenTime= */ any(),
/* currentTime= */ any());
/* currentTime= */ any(),
/* customSignals= */ any());
}

/**
Expand Down Expand Up @@ -919,7 +948,8 @@ private void verifyBackendIsCalled() throws Exception {
/* lastFetchETag= */ any(),
/* customHeaders= */ any(),
/* firstOpenTime= */ any(),
/* currentTime= */ any());
/* currentTime= */ any(),
/* customSignals= */ any());
}

private void verifyBackendIsCalled(Map<String, String> userProperties, Long firstOpenTime)
Expand All @@ -933,7 +963,8 @@ private void verifyBackendIsCalled(Map<String, String> userProperties, Long firs
/* lastFetchETag= */ any(),
/* customHeaders= */ any(),
/* firstOpenTime= */ eq(firstOpenTime),
/* currentTime= */ any());
/* currentTime= */ any(),
/* customSignals= */ any());
}

private void verifyBackendIsNeverCalled() throws Exception {
Expand All @@ -946,7 +977,8 @@ private void verifyBackendIsNeverCalled() throws Exception {
/* lastFetchETag= */ any(),
/* customHeaders= */ any(),
/* firstOpenTime= */ any(),
/* currentTime= */ any());
/* currentTime= */ any(),
/* customSignals= */ any());
}

private void verifyETags(@Nullable String requestETag, String responseETag) throws Exception {
Expand All @@ -959,7 +991,8 @@ private void verifyETags(@Nullable String requestETag, String responseETag) thro
/* lastFetchETag= */ eq(requestETag),
/* customHeaders= */ any(),
/* firstOpenTime= */ any(),
/* currentTime= */ any());
/* currentTime= */ any(),
/* customSignals= */ any());
assertThat(sharedPrefsClient.getLastFetchETag()).isEqualTo(responseETag);
}

Expand Down
28 changes: 12 additions & 16 deletions ...ig/src/test/java/com/google/firebase/remoteconfig/internal/ConfigFetchHttpClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import static com.google.firebase.remoteconfig.RemoteConfigConstants.RequestFieldKey.APP_ID;
import static com.google.firebase.remoteconfig.RemoteConfigConstants.RequestFieldKey.APP_VERSION;
import static com.google.firebase.remoteconfig.RemoteConfigConstants.RequestFieldKey.COUNTRY_CODE;
import static com.google.firebase.remoteconfig.RemoteConfigConstants.RequestFieldKey.CUSTOM_SIGNALS;
import static com.google.firebase.remoteconfig.RemoteConfigConstants.RequestFieldKey.FIRST_OPEN_TIME;
import static com.google.firebase.remoteconfig.RemoteConfigConstants.RequestFieldKey.INSTANCE_ID;
import static com.google.firebase.remoteconfig.RemoteConfigConstants.RequestFieldKey.INSTANCE_ID_TOKEN;
Expand Down Expand Up @@ -86,10 +85,6 @@ public class ConfigFetchHttpClientTest {
"etag-" + PROJECT_NUMBER + "-" + DEFAULT_NAMESPACE + "-fetch-%d";
private static final String FIRST_ETAG = String.format(ETAG_FORMAT, 1);
private static final String SECOND_ETAG = String.format(ETAG_FORMAT, 2);
private static final Map<String, String> SAMPLE_CUSTOM_SIGNALS =
ImmutableMap.of(
"subscription", "premium",
"age", "20");

private Context context;
private ConfigFetchHttpClient configFetchHttpClient;
Expand All @@ -110,8 +105,7 @@ public void setUp() throws Exception {
API_KEY,
DEFAULT_NAMESPACE,
/* connectTimeoutInSeconds= */ 10L,
/* readTimeoutInSeconds= */ 10L,
/* customSignals= */ SAMPLE_CUSTOM_SIGNALS);
/* readTimeoutInSeconds= */ 10L);

hasChangeResponseBody =
new JSONObject()
Expand Down Expand Up @@ -244,8 +238,6 @@ public void fetch_setsAllElementsOfRequestBody_sendsRequestBodyToServer() throws
assertThat(requestBody.get(FIRST_OPEN_TIME)).isEqualTo(firstOpenTimeIsoString);
assertThat(requestBody.getJSONObject(ANALYTICS_USER_PROPERTIES).toString())
.isEqualTo(new JSONObject(customUserProperties).toString());
assertThat(requestBody.getJSONObject(CUSTOM_SIGNALS).toString())
.isEqualTo(new JSONObject(SAMPLE_CUSTOM_SIGNALS).toString());
}

@Test
Expand Down Expand Up @@ -324,8 +316,7 @@ public void fetch_setsTimeouts_urlConnectionHasTimeouts() throws Exception {
API_KEY,
DEFAULT_NAMESPACE,
/* connectTimeoutInSeconds= */ 15L,
/* readTimeoutInSeconds= */ 20L,
/* customSignals= */ SAMPLE_CUSTOM_SIGNALS);
/* readTimeoutInSeconds= */ 20L);
setServerResponseTo(noChangeResponseBody, SECOND_ETAG);

fetch(FIRST_ETAG);
Expand Down Expand Up @@ -353,7 +344,8 @@ private FetchResponse fetch(String eTag) throws Exception {
eTag,
/* customHeaders= */ ImmutableMap.of(),
/* firstOpenTime= */ null,
/* currentTime= */ new Date(mockClock.currentTimeMillis()));
/* currentTime= */ new Date(mockClock.currentTimeMillis()),
/* customSignals= */ ImmutableMap.of());
}

private FetchResponse fetch(String eTag, Map<String, String> userProperties, Long firstOpenTime)
Expand All @@ -366,7 +358,8 @@ private FetchResponse fetch(String eTag, Map<String, String> userProperties, Lon
eTag,
/* customHeaders= */ ImmutableMap.of(),
firstOpenTime,
new Date(mockClock.currentTimeMillis()));
new Date(mockClock.currentTimeMillis()),
/* customSignals= */ ImmutableMap.of());
}

private FetchResponse fetch(String eTag, Map<String, String> customHeaders) throws Exception {
Expand All @@ -378,7 +371,8 @@ private FetchResponse fetch(String eTag, Map<String, String> customHeaders) thro
eTag,
customHeaders,
/* firstOpenTime= */ null,
new Date(mockClock.currentTimeMillis()));
new Date(mockClock.currentTimeMillis()),
/* customSignals= */ ImmutableMap.of());
}

private FetchResponse fetchWithoutInstallationId() throws Exception {
Expand All @@ -390,7 +384,8 @@ private FetchResponse fetchWithoutInstallationId() throws Exception {
/* lastFetchETag= */ "bogus-etag",
/* customHeaders= */ ImmutableMap.of(),
/* firstOpenTime= */ null,
new Date(mockClock.currentTimeMillis()));
new Date(mockClock.currentTimeMillis()),
/* customSignals= */ ImmutableMap.of());
}

private FetchResponse fetchWithoutInstallationAuthToken() throws Exception {
Expand All @@ -402,7 +397,8 @@ private FetchResponse fetchWithoutInstallationAuthToken() throws Exception {
/* lastFetchETag= */ "bogus-etag",
/* customHeaders= */ ImmutableMap.of(),
/* firstOpenTime= */ null,
new Date(mockClock.currentTimeMillis()));
new Date(mockClock.currentTimeMillis()),
/* customSignals= */ ImmutableMap.of());
}

private void setServerResponseTo(JSONObject requestBody, String eTag) {
Expand Down
Loading