Skip to content

Add first-open time to Remote Config fetch request. #3653

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 4 commits into from
Apr 19, 2022
Merged
Show file tree
Hide file tree
Changes from 3 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 @@ -48,7 +48,8 @@ public final class RemoteConfigConstants {
RequestFieldKey.APP_VERSION,
RequestFieldKey.PACKAGE_NAME,
RequestFieldKey.SDK_VERSION,
RequestFieldKey.ANALYTICS_USER_PROPERTIES
RequestFieldKey.ANALYTICS_USER_PROPERTIES,
RequestFieldKey.FIRST_OPEN_TIME
})
@Retention(RetentionPolicy.SOURCE)
public @interface RequestFieldKey {
Expand All @@ -64,6 +65,7 @@ public final class RemoteConfigConstants {
String PACKAGE_NAME = "packageName";
String SDK_VERSION = "sdkVersion";
String ANALYTICS_USER_PROPERTIES = "analyticsUserProperties";
String FIRST_OPEN_TIME = "firstOpenTime";
}

/** Keys of fields in the Fetch response body from the Firebase Remote Config server. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,12 @@ public class ConfigFetchHandler {
*/
@VisibleForTesting static final int HTTP_TOO_MANY_REQUESTS = 429;

/**
* First-open-time key name in GA user-properties. First-open time is a predefined user-dimension
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small nit, looks like you have "first-open time" for the most part in the comments so I'd make it consistent here, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. :)

* automatically collected by GA.
*/
@VisibleForTesting static final String FIRST_OPEN_TIME_KEY = "_fot";

private final FirebaseInstallationsApi firebaseInstallations;
private final Provider<AnalyticsConnector> analyticsConnector;

Expand Down Expand Up @@ -110,7 +116,6 @@ public ConfigFetchHandler(
this.fetchedConfigsCache = fetchedConfigsCache;
this.frcBackendApiClient = frcBackendApiClient;
this.frcMetadata = frcMetadata;

this.customHttpHeaders = customHttpHeaders;
}

Expand Down Expand Up @@ -311,6 +316,7 @@ private FetchResponse fetchFromBackend(
getUserProperties(),
frcMetadata.getLastFetchETag(),
customHttpHeaders,
getFirstOpenTime(),
currentTime);

if (response.getLastFetchETag() != null) {
Expand Down Expand Up @@ -492,8 +498,8 @@ private void updateLastFetchStatusAndTime(
}

/**
* Returns the list of user properties in Analytics. If the Analytics SDK is not available,
* returns an empty list.
* Returns the list of custom user properties in Analytics. If the Analytics SDK is not available,
* this method returns an empty list.
*/
@WorkerThread
private Map<String, String> getUserProperties() {
Expand All @@ -510,6 +516,20 @@ private Map<String, String> getUserProperties() {
return userPropertiesMap;
}

/**
* Returns first-open time from Analytics. If the Analytics SDK is not available, or if Analytics
* does not have first-open time for the app, this method returns null.
*/
@WorkerThread
private Long getFirstOpenTime() {
AnalyticsConnector connector = this.analyticsConnector.get();
if (connector == null) {
return null;
}

return (Long) connector.getUserProperties(/*includeInternal=*/ true).get(FIRST_OPEN_TIME_KEY);
}

/** Used to verify that the fetch handler is getting Analytics as expected. */
@VisibleForTesting
public Provider<AnalyticsConnector> getAnalyticsConnector() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
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.FIRST_OPEN_TIME;
import static com.google.firebase.remoteconfig.RemoteConfigConstants.RequestFieldKey.INSTANCE_ID;
import static com.google.firebase.remoteconfig.RemoteConfigConstants.RequestFieldKey.INSTANCE_ID_TOKEN;
import static com.google.firebase.remoteconfig.RemoteConfigConstants.RequestFieldKey.LANGUAGE_CODE;
Expand Down Expand Up @@ -58,6 +59,7 @@
import java.net.HttpURLConnection;
import java.net.URL;
import java.net.URLConnection;
import java.text.SimpleDateFormat;
import java.util.Date;
import java.util.HashMap;
import java.util.Locale;
Expand Down Expand Up @@ -92,6 +94,9 @@ public class ConfigFetchHttpClient {
private final long connectTimeoutInSeconds;
private final long readTimeoutInSeconds;

/** ISO-8601 UTC timestamp format. */
private static final String ISO_DATE_PATTERN = "yyyy-MM-dd'T'HH:mm:ss.SSS'Z'";

/** Creates a client for {@link #fetch}ing data from the Firebase Remote Config server. */
public ConfigFetchHttpClient(
Context context,
Expand Down Expand Up @@ -163,6 +168,7 @@ HttpURLConnection createHttpURLConnection() throws FirebaseRemoteConfigException
* server uses this ETag to determine if there has been a change in the response body since
* the last fetch.
* @param customHeaders custom HTTP headers that will be sent to the FRC server.
* @param firstOpenTime first time the app was opened. This value comes from Google Analytics.
* @param currentTime the current time on the device that is performing the fetch.
*/
// TODO(issues/263): Set custom headers in ConfigFetchHttpClient's constructor.
Expand All @@ -174,6 +180,7 @@ FetchResponse fetch(
Map<String, String> analyticsUserProperties,
String lastFetchETag,
Map<String, String> customHeaders,
Long firstOpenTime,
Date currentTime)
throws FirebaseRemoteConfigException {
setUpUrlConnection(urlConnection, lastFetchETag, installationAuthToken, customHeaders);
Expand All @@ -182,7 +189,8 @@ FetchResponse fetch(
JSONObject fetchResponse;
try {
byte[] requestBody =
createFetchRequestBody(installationId, installationAuthToken, analyticsUserProperties)
createFetchRequestBody(
installationId, installationAuthToken, analyticsUserProperties, firstOpenTime)
.toString()
.getBytes("utf-8");
setFetchRequestBody(urlConnection, requestBody);
Expand Down Expand Up @@ -292,7 +300,8 @@ private String getFingerprintHashForPackage() {
private JSONObject createFetchRequestBody(
String installationId,
String installationAuthToken,
Map<String, String> analyticsUserProperties)
Map<String, String> analyticsUserProperties,
Long firstOpenTime)
throws FirebaseRemoteConfigClientException {
Map<String, Object> requestBodyMap = new HashMap<>();

Expand All @@ -315,7 +324,7 @@ private JSONObject createFetchRequestBody(
: locale.toString();
requestBodyMap.put(LANGUAGE_CODE, languageCode);

requestBodyMap.put(PLATFORM_VERSION, Integer.toString(android.os.Build.VERSION.SDK_INT));
requestBodyMap.put(PLATFORM_VERSION, Integer.toString(Build.VERSION.SDK_INT));

requestBodyMap.put(TIME_ZONE, TimeZone.getDefault().getID());

Expand All @@ -336,9 +345,19 @@ private JSONObject createFetchRequestBody(

requestBodyMap.put(ANALYTICS_USER_PROPERTIES, new JSONObject(analyticsUserProperties));

if (firstOpenTime != null) {
requestBodyMap.put(FIRST_OPEN_TIME, convertToISOString(firstOpenTime));
}

return new JSONObject(requestBodyMap);
}

private String convertToISOString(long millisFromEpoch) {
SimpleDateFormat isoDateFormat = new SimpleDateFormat(ISO_DATE_PATTERN);
isoDateFormat.setTimeZone(TimeZone.getTimeZone("UTC"));
return isoDateFormat.format(millisFromEpoch);
}

private void setFetchRequestBody(HttpURLConnection urlConnection, byte[] requestBody)
throws IOException {
urlConnection.setFixedLengthStreamingMode(requestBody.length);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import static com.google.firebase.remoteconfig.RemoteConfigConstants.ResponseFieldKey.STATE;
import static com.google.firebase.remoteconfig.internal.ConfigFetchHandler.BACKOFF_TIME_DURATIONS_IN_MINUTES;
import static com.google.firebase.remoteconfig.internal.ConfigFetchHandler.DEFAULT_MINIMUM_FETCH_INTERVAL_IN_SECONDS;
import static com.google.firebase.remoteconfig.internal.ConfigFetchHandler.FIRST_OPEN_TIME_KEY;
import static com.google.firebase.remoteconfig.internal.ConfigFetchHandler.HTTP_TOO_MANY_REQUESTS;
import static com.google.firebase.remoteconfig.internal.ConfigMetadataClient.LAST_FETCH_TIME_NO_FETCH_YET;
import static com.google.firebase.remoteconfig.internal.ConfigMetadataClient.NO_BACKOFF_TIME;
Expand All @@ -42,6 +43,7 @@
import static java.util.concurrent.TimeUnit.MINUTES;
import static java.util.concurrent.TimeUnit.SECONDS;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.doThrow;
Expand Down Expand Up @@ -195,6 +197,7 @@ public void fetch_firstFetch_includesInstallationAuthToken() throws Exception {
/* analyticsUserProperties= */ any(),
/* lastFetchETag= */ any(),
/* customHeaders= */ any(),
/* firstOpenTime= */ any(),
/* currentTime= */ any());
}

Expand Down Expand Up @@ -394,7 +397,7 @@ 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()))
when(mockBackendFetchApiClient.fetch(any(), any(), any(), any(), any(), any(), any(), any()))
.thenThrow(
new FirebaseRemoteConfigClientException("Fetch failed due to an unexpected error."));

Expand Down Expand Up @@ -628,21 +631,45 @@ public void fetch_serverReturnsUnexpectedCode_throwsServerException() throws Exc
}

@Test
public void fetch_hasAnalyticsSdk_sendsUserProperties() throws Exception {
public void fetch_hasAnalyticsSdk_sendsUserPropertiesAndFirstOpenTime() throws Exception {
// Provide the mock Analytics SDK.
AnalyticsConnector mockAnalyticsConnector = mock(AnalyticsConnector.class);
fetchHandler = getNewFetchHandler(mockAnalyticsConnector);
long firstOpenTime = 1636146000000L;

Map<String, String> userProperties =
Map<String, String> customUserProperties =
ImmutableMap.of("up_key1", "up_val1", "up_key2", "up_val2");
Map<String, Object> allUserProperties =
ImmutableMap.of(
"up_key1", "up_val1", "up_key2", "up_val2", FIRST_OPEN_TIME_KEY, firstOpenTime);
when(mockAnalyticsConnector.getUserProperties(/*includeInternal=*/ false))
.thenReturn(ImmutableMap.copyOf(customUserProperties));
when(mockAnalyticsConnector.getUserProperties(/*includeInternal=*/ true))
.thenReturn(ImmutableMap.copyOf(allUserProperties));

fetchCallToHttpClientUpdatesClockAndReturnsConfig(firstFetchedContainer);

assertWithMessage("Fetch() failed!").that(fetchHandler.fetch().isSuccessful()).isTrue();

verifyBackendIsCalled(customUserProperties, firstOpenTime);
}

@Test
public void fetch_hasAnalyticsSdk_sendsUserPropertiesNoFirstOpenTime() throws Exception {
// Provide the mock Analytics SDK.
AnalyticsConnector mockAnalyticsConnector = mock(AnalyticsConnector.class);
fetchHandler = getNewFetchHandler(mockAnalyticsConnector);

Map<String, String> userProperties =
ImmutableMap.of("up_key1", "up_val1", "up_key2", "up_val2");
when(mockAnalyticsConnector.getUserProperties(anyBoolean()))
.thenReturn(ImmutableMap.copyOf(userProperties));

fetchCallToHttpClientUpdatesClockAndReturnsConfig(firstFetchedContainer);

assertWithMessage("Fetch() failed!").that(fetchHandler.fetch().isSuccessful()).isTrue();

verifyBackendIsCalled(userProperties);
verifyBackendIsCalled(userProperties, null);
}

@Test
Expand Down Expand Up @@ -751,6 +778,7 @@ private void setBackendResponseConfigsTo(ConfigContainer container) throws Excep
/* analyticsUserProperties= */ any(),
/* lastFetchETag= */ any(),
/* customHeaders= */ any(),
/* firstOpenTime= */ any(),
/* currentTime= */ any());
}

Expand All @@ -762,6 +790,7 @@ private void setBackendResponseToNoChange(Date date) throws Exception {
/* analyticsUserProperties= */ any(),
/* lastFetchETag= */ any(),
/* customHeaders= */ any(),
/* firstOpenTime= */ any(),
/* currentTime= */ any()))
.thenReturn(FetchResponse.forBackendHasNoUpdates(date));
}
Expand All @@ -776,6 +805,7 @@ private void fetchCallToBackendThrowsException(int httpErrorCode) throws Excepti
/* analyticsUserProperties= */ any(),
/* lastFetchETag= */ any(),
/* customHeaders= */ any(),
/* firstOpenTime= */ any(),
/* currentTime= */ any());
}

Expand Down Expand Up @@ -855,10 +885,12 @@ private void verifyBackendIsCalled() throws Exception {
/* analyticsUserProperties= */ any(),
/* lastFetchETag= */ any(),
/* customHeaders= */ any(),
/* firstOpenTime= */ any(),
/* currentTime= */ any());
}

private void verifyBackendIsCalled(Map<String, String> userProperties) throws Exception {
private void verifyBackendIsCalled(Map<String, String> userProperties, Long firstOpenTime)
throws Exception {
verify(mockBackendFetchApiClient)
.fetch(
any(HttpURLConnection.class),
Expand All @@ -867,6 +899,7 @@ private void verifyBackendIsCalled(Map<String, String> userProperties) throws Ex
/* analyticsUserProperties= */ eq(userProperties),
/* lastFetchETag= */ any(),
/* customHeaders= */ any(),
/* firstOpenTime= */ eq(firstOpenTime),
/* currentTime= */ any());
}

Expand All @@ -879,6 +912,7 @@ private void verifyBackendIsNeverCalled() throws Exception {
/* analyticsUserProperties= */ any(),
/* lastFetchETag= */ any(),
/* customHeaders= */ any(),
/* firstOpenTime= */ any(),
/* currentTime= */ any());
}

Expand All @@ -891,6 +925,7 @@ private void verifyETags(@Nullable String requestETag, String responseETag) thro
/* analyticsUserProperties= */ any(),
/* lastFetchETag= */ eq(requestETag),
/* customHeaders= */ any(),
/* firstOpenTime= */ any(),
/* currentTime= */ any());
assertThat(metadataClient.getLastFetchETag()).isEqualTo(responseETag);
}
Expand Down
Loading