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.

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

Tushar khandelwal/rc custom targeting #6410

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open
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
1 change: 1 addition & 0 deletions firebase-config/api.txt
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ package com.google.firebase.remoteconfig {
method @NonNull public com.google.firebase.remoteconfig.FirebaseRemoteConfigValue getValue(@NonNull String);
method @NonNull public com.google.android.gms.tasks.Task<java.lang.Void> reset();
method @NonNull public com.google.android.gms.tasks.Task<java.lang.Void> setConfigSettingsAsync(@NonNull com.google.firebase.remoteconfig.FirebaseRemoteConfigSettings);
method @NonNull public com.google.android.gms.tasks.Task<java.lang.Void> setCustomSignals(@NonNull java.util.Map<java.lang.String,java.lang.Object>);
method @NonNull public com.google.android.gms.tasks.Task<java.lang.Void> setDefaultsAsync(@NonNull java.util.Map<java.lang.String,java.lang.Object>);
method @NonNull public com.google.android.gms.tasks.Task<java.lang.Void> setDefaultsAsync(@XmlRes int);
field public static final boolean DEFAULT_VALUE_FOR_BOOLEAN = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,28 @@ private Task<Void> setDefaultsWithStringsMapAsync(Map<String, String> defaultsSt
FirebaseExecutors.directExecutor(), (unusedContainer) -> Tasks.forResult(null));
}

/**
* Asynchronously changes the custom signals for this {@link FirebaseRemoteConfig} instance.
*
* <p>The values in {@code customSignals} must be one of the following types:
*
* <ul>
* <li><code>Long</code>
* <li><code>String</code>
* </ul>
*
* @param customSignals Map (key, value) of the custom signals to be set for the app instance
*/
@NonNull
public Task<Void> setCustomSignals(@NonNull Map<String, Object> customSignals) {
return Tasks.call(
executor,
() -> {
frcMetadata.setCustomSignals(customSignals);
return null;
});
}

/**
* Notifies the Firebase A/B Testing SDK about activated experiments.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,8 @@ ConfigFetchHttpClient getFrcBackendApiClient(
apiKey,
namespace,
/* connectTimeoutInSeconds= */ metadataClient.getFetchTimeoutInSeconds(),
/* readTimeoutInSeconds= */ metadataClient.getFetchTimeoutInSeconds());
/* readTimeoutInSeconds= */ metadataClient.getFetchTimeoutInSeconds(),
/* customSignals= */ metadataClient.getCustomSignals());
}

@VisibleForTesting
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ public final class RemoteConfigConstants {
RequestFieldKey.PACKAGE_NAME,
RequestFieldKey.SDK_VERSION,
RequestFieldKey.ANALYTICS_USER_PROPERTIES,
RequestFieldKey.FIRST_OPEN_TIME
RequestFieldKey.FIRST_OPEN_TIME,
RequestFieldKey.CUSTOM_SIGNALS
})
@Retention(RetentionPolicy.SOURCE)
public @interface RequestFieldKey {
Expand All @@ -68,6 +69,7 @@ public final class RemoteConfigConstants {
String SDK_VERSION = "sdkVersion";
String ANALYTICS_USER_PROPERTIES = "analyticsUserProperties";
String FIRST_OPEN_TIME = "firstOpenTime";
String CUSTOM_SIGNALS = "customSignals";
}

/** 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 @@ -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.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 @@ -93,6 +94,7 @@ public class ConfigFetchHttpClient {
private final String apiKey;
private final String projectNumber;
private final String namespace;
Map<String, Object> customSignalMap;
private final long connectTimeoutInSeconds;
private final long readTimeoutInSeconds;

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

/** Used to verify that the timeout is being set correctly. */
Expand Down Expand Up @@ -347,6 +351,14 @@ private JSONObject createFetchRequestBody(

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

if (!customSignalMap.isEmpty()) {
Map<String, String> customSignalsStringMap = new HashMap<>();
for (Map.Entry<String, Object> entry : customSignalMap.entrySet()) {
customSignalsStringMap.put(entry.getKey(), String.valueOf(entry.getValue()));
}
requestBodyMap.put(CUSTOM_SIGNALS, new JSONObject(customSignalsStringMap));
}

if (firstOpenTime != null) {
requestBodyMap.put(FIRST_OPEN_TIME, convertToISOString(firstOpenTime));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,14 @@
import static com.google.firebase.remoteconfig.FirebaseRemoteConfig.LAST_FETCH_STATUS_NO_FETCH_YET;
import static com.google.firebase.remoteconfig.FirebaseRemoteConfig.LAST_FETCH_STATUS_SUCCESS;
import static com.google.firebase.remoteconfig.FirebaseRemoteConfig.LAST_FETCH_STATUS_THROTTLED;
import static com.google.firebase.remoteconfig.FirebaseRemoteConfig.TAG;
import static com.google.firebase.remoteconfig.RemoteConfigComponent.CONNECTION_TIMEOUT_IN_SECONDS;
import static com.google.firebase.remoteconfig.RemoteConfigConstants.RequestFieldKey.CUSTOM_SIGNALS;
import static com.google.firebase.remoteconfig.internal.ConfigFetchHandler.DEFAULT_MINIMUM_FETCH_INTERVAL_IN_SECONDS;
import static java.lang.annotation.RetentionPolicy.SOURCE;

import android.content.SharedPreferences;
import android.util.Log;
import androidx.annotation.IntDef;
import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting;
Expand All @@ -31,6 +34,11 @@
import com.google.firebase.remoteconfig.FirebaseRemoteConfigSettings;
import java.lang.annotation.Retention;
import java.util.Date;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
import org.json.JSONException;
import org.json.JSONObject;

/**
* Client for handling Firebase Remote Config (FRC) metadata that is saved to disk and persisted
Expand Down Expand Up @@ -75,17 +83,25 @@ public class ConfigMetadataClient {
private static final String REALTIME_BACKOFF_END_TIME_IN_MILLIS_KEY =
"realtime_backoff_end_time_in_millis";

/** Constants for custom signal limits. */
private static final int CUSTOM_SIGNALS_MAX_KEY_LENGTH = 250;

private static final int CUSTOM_SIGNALS_MAX_STRING_VALUE_LENGTH = 500;
private static final int CUSTOM_SIGNALS_MAX_COUNT = 100;

private final SharedPreferences frcMetadata;

private final Object frcInfoLock;
private final Object backoffMetadataLock;
private final Object realtimeBackoffMetadataLock;
private final Object customSignalsLock;

public ConfigMetadataClient(SharedPreferences frcMetadata) {
this.frcMetadata = frcMetadata;
this.frcInfoLock = new Object();
this.backoffMetadataLock = new Object();
this.realtimeBackoffMetadataLock = new Object();
this.customSignalsLock = new Object();
}

public long getFetchTimeoutInSeconds() {
Expand Down Expand Up @@ -249,6 +265,80 @@ void setBackoffMetadata(int numFailedFetches, Date backoffEndTime) {
}
}

public void setCustomSignals(Map<String, Object> newCustomSignals) {
synchronized (customSignalsLock) {
// Retrieve existing custom signals
Map<String, Object> existingCustomSignals = getCustomSignals();

for (Map.Entry<String, Object> entry : newCustomSignals.entrySet()) {
String key = entry.getKey();
Object value = entry.getValue();

// Validate value type, and key and value length
if (value != null && !(value instanceof String || value instanceof Long)) {
Log.w(TAG, "Invalid custom signal: Custom signal values must be of type String or Long.");
return;
}
if (key.length() > CUSTOM_SIGNALS_MAX_KEY_LENGTH
|| (value instanceof String
&& ((String) value).length() > CUSTOM_SIGNALS_MAX_STRING_VALUE_LENGTH)) {
Log.w(
TAG,
String.format(
"Invalid custom signal: Custom signal keys must be %d characters or less, and string values must be %d characters or less.",
CUSTOM_SIGNALS_MAX_KEY_LENGTH, CUSTOM_SIGNALS_MAX_STRING_VALUE_LENGTH));
return;
}

// Merge new signals with existing ones, overwriting existing keys.
// Also, remove entries where the new value is null.
if (value != null) {
existingCustomSignals.put(key, value);
} else {
existingCustomSignals.remove(key);
}
}

// Check if the map has actually changed and the size limit
if (existingCustomSignals.equals(getCustomSignals())) {
return;
}
if (existingCustomSignals.size() > CUSTOM_SIGNALS_MAX_COUNT) {
Log.w(
TAG,
String.format(
"Invalid custom signal: Too many custom signals provided. The maximum allowed is %d.",
CUSTOM_SIGNALS_MAX_COUNT));
return;
}

frcMetadata
.edit()
.putString(CUSTOM_SIGNALS, new JSONObject(existingCustomSignals).toString())
.commit();
}
}

public Map<String, Object> getCustomSignals() {
String jsonString = frcMetadata.getString(CUSTOM_SIGNALS, "{}");
try {
JSONObject existingCustomSignalsJson = new JSONObject(jsonString);
Map<String, Object> custom_signals = new HashMap<>();
Iterator<String> keys = existingCustomSignalsJson.keys();
while (keys.hasNext()) {
String key = keys.next();
Object value = existingCustomSignalsJson.get(key);
if (value instanceof Integer) {
value = existingCustomSignalsJson.getLong(key);
}
custom_signals.put(key, value);
}
return custom_signals;
} catch (JSONException e) {
return new HashMap<>();
}
}

void resetBackoff() {
setBackoffMetadata(NO_FAILED_FETCHES, NO_BACKOFF_TIME);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,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.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 @@ -85,6 +86,10 @@ 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, Object> SAMPLE_CUSTOM_SIGNALS =
ImmutableMap.of(
"subscription", "premium",
"age", "20");

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

hasChangeResponseBody =
new JSONObject()
Expand Down Expand Up @@ -238,6 +244,8 @@ 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 @@ -316,7 +324,8 @@ public void fetch_setsTimeouts_urlConnectionHasTimeouts() throws Exception {
API_KEY,
DEFAULT_NAMESPACE,
/* connectTimeoutInSeconds= */ 15L,
/* readTimeoutInSeconds= */ 20L);
/* readTimeoutInSeconds= */ 20L,
/* customSignals= */ SAMPLE_CUSTOM_SIGNALS);
setServerResponseTo(noChangeResponseBody, SECOND_ETAG);

fetch(FIRST_ETAG);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,21 @@
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;
import static com.google.firebase.remoteconfig.internal.ConfigMetadataClient.NO_FAILED_FETCHES;
import static com.google.firebase.remoteconfig.testutil.Assert.assertFalse;

import android.content.Context;
import android.content.SharedPreferences;
import androidx.test.core.app.ApplicationProvider;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
import com.google.firebase.remoteconfig.FirebaseRemoteConfigInfo;
import com.google.firebase.remoteconfig.FirebaseRemoteConfigSettings;
import com.google.firebase.remoteconfig.internal.ConfigMetadataClient.BackoffMetadata;
import com.google.firebase.remoteconfig.internal.ConfigMetadataClient.LastFetchStatus;
import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
import java.util.Map;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -349,4 +354,55 @@ public void clear_hasSetValues_clearsAll() {
assertThat(info.getConfigSettings().getMinimumFetchIntervalInSeconds())
.isEqualTo(DEFAULT_MINIMUM_FETCH_INTERVAL_IN_SECONDS);
}

@Test
public void getCustomSignals_isNotSet_returnsEmptyMap() {
assertThat(metadataClient.getCustomSignals()).isEqualTo(Collections.emptyMap());
}

@Test
public void getCustomSignals_isSet_returnsCustomSignals() {
Map<String, Object> SAMPLE_CUSTOM_SIGNALS =
ImmutableMap.of(
"subscription", "premium",
"age", "20");
metadataClient.setCustomSignals(SAMPLE_CUSTOM_SIGNALS);
assertThat(metadataClient.getCustomSignals()).isEqualTo(SAMPLE_CUSTOM_SIGNALS);
}

@Test
public void setCustomSignals_multipleTimes_addsNewSignals() {
Map<String, Object> signals1 = ImmutableMap.of("subscription", "premium");
Map<String, Object> signals2 = ImmutableMap.of("age", 20L);
metadataClient.setCustomSignals(signals1);
metadataClient.setCustomSignals(signals2);
Map<String, Object> expectedSignals = ImmutableMap.of("subscription", "premium", "age", 20L);
assertThat(metadataClient.getCustomSignals()).isEqualTo(expectedSignals);
}

@Test
public void setCustomSignals_nullValue_removesSignal() {
Map<String, Object> signals1 = ImmutableMap.of("subscription", "premium", "age", 20L);
metadataClient.setCustomSignals(signals1);
Map<String, Object> signals2 = new HashMap<>();
signals2.put("age", null);
metadataClient.setCustomSignals(signals2);
Map<String, Object> expectedSignals = ImmutableMap.of("subscription", "premium");
assertThat(metadataClient.getCustomSignals()).isEqualTo(expectedSignals);
}

@Test
public void setCustomSignals_invalidInput_rejectsSignals() {
Map<String, Object> invalidSignals1 = ImmutableMap.of("name", true);
metadataClient.setCustomSignals(invalidSignals1);
assertFalse(metadataClient.getCustomSignals().containsKey("name"));

Map<String, Object> invalidSignals2 = ImmutableMap.of("a".repeat(251), "value");
metadataClient.setCustomSignals(invalidSignals2);
assertFalse(metadataClient.getCustomSignals().containsKey("a".repeat(251)));

Map<String, Object> invalidSignals3 = ImmutableMap.of("key", "a".repeat(501));
metadataClient.setCustomSignals(invalidSignals3);
assertFalse(metadataClient.getCustomSignals().containsKey("key"));
}
}
Loading