Skip to content

Commit 46fb82f

Browse files
vic-flairvic-flair
committed
Rename constants.
1 parent 10daf06 commit 46fb82f

File tree

4 files changed

+72
-81
lines changed

4 files changed

+72
-81
lines changed

‎firebase-config/src/main/java/com/google/firebase/remoteconfig/internal/Personalization.java

Lines changed: 26 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -25,26 +25,27 @@
2525
public class Personalization {
2626
public static final String ANALYTICS_ORIGIN_PERSONALIZATION = "fp";
2727

28-
// Constants with LOG_KEY suffix are how the corresponding ones without it are identified on
28+
// Constants with PARAM suffix are how the corresponding ones without it are identified on
2929
// Google Analytics.
30-
public static final String ANALYTICS_PULL_EVENT = "personalization_assignment";
31-
public static final String ARM_KEY = "arm_key";
32-
public static final String ARM_VALUE = "arm_value";
30+
public static final String EXTERNAL_EVENT = "personalization_assignment";
31+
public static final String EXTERNAL_RC_PARAMETER_PARAM = "arm_key";
32+
public static final String EXTERNAL_ARM_VALUE_PARAM = "arm_value";
3333
public static final String PERSONALIZATION_ID = "personalizationId";
34-
public static final String PERSONALIZATION_ID_LOG_KEY = "personalization_id";
34+
public static final String EXTERNAL_PERSONALIZATION_ID_PARAM = "personalization_id";
3535
public static final String ARM_INDEX = "armIndex";
36-
public static final String ARM_INDEX_LOG_KEY = "arm_index";
36+
public static final String EXTERNAL_ARM_INDEX_PARAM = "arm_index";
3737
public static final String GROUP = "group";
38+
public static final String EXTERNAL_GROUP_PARAM = "group";
3839

39-
public static final String ANALYTICS_PULL_EVENT_INTERNAL = "_fpc";
40+
public static final String INTERNAL_EVENT = "_fpc";
4041
public static final String CHOICE_ID = "choiceId";
41-
public static final String CHOICE_ID_LOG_KEY = "_fpid";
42+
public static final String INTERNAL_CHOICE_ID_PARAM = "_fpid";
4243

4344
/** The app's Firebase Analytics client. */
4445
private final AnalyticsConnector analyticsConnector;
4546

4647
/** A map of Remote Config parameter key to choice ID. */
47-
private final Map<String, String> armsCache =
48+
private final Map<String, String> loggedChoiceIds =
4849
Collections.synchronizedMap(new HashMap<String, String>());
4950

5051
/** Creates an instance of {@code Personalization}. */
@@ -56,11 +57,11 @@ public Personalization(@NonNull AnalyticsConnector analyticsConnector) {
5657
* Called when a Personalization parameter value (an arm) is retrieved, and uses Google Analytics
5758
* for Firebase to log metadata if it's a Personalization parameter.
5859
*
59-
* @param key Remote Config parameter
60+
* @param rcParameter Remote Config parameter
6061
* @param configContainer {@link ConfigContainer} containing Personalization metadata for {@code
6162
* key}
6263
*/
63-
public void logArmActive(@NonNull String key, @NonNull ConfigContainer configContainer) {
64+
public void logArmActive(@NonNull String rcParameter, @NonNull ConfigContainer configContainer) {
6465
JSONObject ids = configContainer.getPersonalizationMetadata();
6566
if (ids.length() < 1) {
6667
return;
@@ -71,7 +72,7 @@ public void logArmActive(@NonNull String key, @NonNull ConfigContainer configCon
7172
return;
7273
}
7374

74-
JSONObject metadata = ids.optJSONObject(key);
75+
JSONObject metadata = ids.optJSONObject(rcParameter);
7576
if (metadata == null) {
7677
return;
7778
}
@@ -81,24 +82,26 @@ public void logArmActive(@NonNull String key, @NonNull ConfigContainer configCon
8182
return;
8283
}
8384

84-
synchronized (armsCache) {
85-
if (choiceId.equals(armsCache.get(key))) {
85+
// We only to need to log each choice ID once, so this attempts to prevent too much unnecessary
86+
// logging.
87+
synchronized (loggedChoiceIds) {
88+
if (choiceId.equals(loggedChoiceIds.get(rcParameter))) {
8689
return;
8790
}
88-
armsCache.put(key, choiceId);
91+
loggedChoiceIds.put(rcParameter, choiceId);
8992
}
9093

9194
Bundle logParams = new Bundle();
92-
logParams.putString(ARM_KEY, key);
93-
logParams.putString(ARM_VALUE, values.optString(key));
94-
logParams.putString(PERSONALIZATION_ID_LOG_KEY, metadata.optString(PERSONALIZATION_ID));
95-
logParams.putInt(ARM_INDEX_LOG_KEY, metadata.optInt(ARM_INDEX, -1));
96-
logParams.putString(GROUP, metadata.optString(GROUP));
97-
analyticsConnector.logEvent(ANALYTICS_ORIGIN_PERSONALIZATION, ANALYTICS_PULL_EVENT, logParams);
95+
logParams.putString(EXTERNAL_RC_PARAMETER_PARAM, rcParameter);
96+
logParams.putString(EXTERNAL_ARM_VALUE_PARAM, values.optString(rcParameter));
97+
logParams.putString(EXTERNAL_PERSONALIZATION_ID_PARAM, metadata.optString(PERSONALIZATION_ID));
98+
logParams.putInt(EXTERNAL_ARM_INDEX_PARAM, metadata.optInt(ARM_INDEX, -1));
99+
logParams.putString(EXTERNAL_GROUP_PARAM, metadata.optString(GROUP));
100+
analyticsConnector.logEvent(ANALYTICS_ORIGIN_PERSONALIZATION, EXTERNAL_EVENT, logParams);
98101

99102
Bundle internalLogParams = new Bundle();
100-
internalLogParams.putString(CHOICE_ID_LOG_KEY, choiceId);
103+
internalLogParams.putString(INTERNAL_CHOICE_ID_PARAM, choiceId);
101104
analyticsConnector.logEvent(
102-
ANALYTICS_ORIGIN_PERSONALIZATION, ANALYTICS_PULL_EVENT_INTERNAL, internalLogParams);
105+
ANALYTICS_ORIGIN_PERSONALIZATION, INTERNAL_EVENT, internalLogParams);
103106
}
104107
}

‎firebase-config/src/test/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigTest.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -958,7 +958,7 @@ public void personalization_hasMetadata_successful() throws Exception {
958958
.when(mockAnalyticsConnector)
959959
.logEvent(
960960
eq(Personalization.ANALYTICS_ORIGIN_PERSONALIZATION),
961-
eq(Personalization.ANALYTICS_PULL_EVENT),
961+
eq(Personalization.EXTERNAL_EVENT),
962962
any(Bundle.class));
963963

964964
ConfigContainer configContainer =
@@ -986,18 +986,18 @@ public void personalization_hasMetadata_successful() throws Exception {
986986
verify(mockAnalyticsConnector, times(2))
987987
.logEvent(
988988
eq(Personalization.ANALYTICS_ORIGIN_PERSONALIZATION),
989-
eq(Personalization.ANALYTICS_PULL_EVENT),
989+
eq(Personalization.EXTERNAL_EVENT),
990990
any(Bundle.class));
991991
assertThat(fakeLogs).hasSize(2);
992992

993993
Bundle params1 = new Bundle();
994-
params1.putString(Personalization.ARM_KEY, "id1");
995-
params1.putString(Personalization.ARM_VALUE, "value1");
994+
params1.putString(Personalization.EXTERNAL_RC_PARAMETER_PARAM, "id1");
995+
params1.putString(Personalization.EXTERNAL_ARM_VALUE_PARAM, "value1");
996996
assertThat(fakeLogs.get(0).toString()).isEqualTo(params1.toString());
997997

998998
Bundle params2 = new Bundle();
999-
params2.putString(Personalization.ARM_KEY, "id2");
1000-
params2.putString(Personalization.ARM_VALUE, "value2");
999+
params2.putString(Personalization.EXTERNAL_RC_PARAMETER_PARAM, "id2");
1000+
params2.putString(Personalization.EXTERNAL_ARM_VALUE_PARAM, "value2");
10011001
assertThat(fakeLogs.get(1).toString()).isEqualTo(params2.toString());
10021002
});
10031003
}

‎firebase-config/src/test/java/com/google/firebase/remoteconfig/internal/ConfigStorageClientTest.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,8 @@ public void read_validContainerWithPersonalization_returnsContainer() throws Exc
9090
ConfigContainer configWithPersonalization =
9191
ConfigContainer.newBuilder(configContainer)
9292
.withPersonalizationMetadata(
93-
new JSONObject(ImmutableMap.of(Personalization.ARM_KEY, "arm_value")))
93+
new JSONObject(
94+
ImmutableMap.of(Personalization.EXTERNAL_ARM_VALUE_PARAM, "arm_value")))
9495
.build();
9596
storageClient.write(configWithPersonalization);
9697
Preconditions.checkArgument(getFileAsString().equals(configWithPersonalization.toString()));

‎firebase-config/src/test/java/com/google/firebase/remoteconfig/internal/PersonalizationTest.java

Lines changed: 38 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,14 @@
1616

1717
import static com.google.common.truth.Truth.assertThat;
1818
import static com.google.firebase.remoteconfig.internal.Personalization.ANALYTICS_ORIGIN_PERSONALIZATION;
19-
import static com.google.firebase.remoteconfig.internal.Personalization.ANALYTICS_PULL_EVENT;
20-
import static com.google.firebase.remoteconfig.internal.Personalization.ANALYTICS_PULL_EVENT_INTERNAL;
21-
import static com.google.firebase.remoteconfig.internal.Personalization.ARM_INDEX_LOG_KEY;
22-
import static com.google.firebase.remoteconfig.internal.Personalization.ARM_KEY;
23-
import static com.google.firebase.remoteconfig.internal.Personalization.ARM_VALUE;
24-
import static com.google.firebase.remoteconfig.internal.Personalization.CHOICE_ID_LOG_KEY;
25-
import static com.google.firebase.remoteconfig.internal.Personalization.GROUP;
26-
import static com.google.firebase.remoteconfig.internal.Personalization.PERSONALIZATION_ID_LOG_KEY;
19+
import static com.google.firebase.remoteconfig.internal.Personalization.EXTERNAL_EVENT;
20+
import static com.google.firebase.remoteconfig.internal.Personalization.INTERNAL_EVENT;
21+
import static com.google.firebase.remoteconfig.internal.Personalization.EXTERNAL_ARM_INDEX_PARAM;
22+
import static com.google.firebase.remoteconfig.internal.Personalization.EXTERNAL_RC_PARAMETER_PARAM;
23+
import static com.google.firebase.remoteconfig.internal.Personalization.EXTERNAL_ARM_VALUE_PARAM;
24+
import static com.google.firebase.remoteconfig.internal.Personalization.INTERNAL_CHOICE_ID_PARAM;
25+
import static com.google.firebase.remoteconfig.internal.Personalization.EXTERNAL_GROUP_PARAM;
26+
import static com.google.firebase.remoteconfig.internal.Personalization.EXTERNAL_PERSONALIZATION_ID_PARAM;
2727
import static org.mockito.AdditionalMatchers.or;
2828
import static org.mockito.ArgumentMatchers.any;
2929
import static org.mockito.ArgumentMatchers.eq;
@@ -52,6 +52,10 @@
5252
@Config(manifest = Config.NONE)
5353
public class PersonalizationTest {
5454
private static final ConfigContainer CONFIG_CONTAINER;
55+
private static final Bundle LOG_PARAMS_1 = new Bundle();
56+
private static final Bundle LOG_PARAMS_2 = new Bundle();
57+
private static final Bundle INTERNAL_LOG_PARAMS_1 = new Bundle();
58+
private static final Bundle INTERNAL_LOG_PARAMS_2 = new Bundle();
5559

5660
static {
5761
try {
@@ -70,6 +74,22 @@ public class PersonalizationTest {
7074
} catch (JSONException e) {
7175
throw new RuntimeException(e);
7276
}
77+
78+
LOG_PARAMS_1.putString(EXTERNAL_RC_PARAMETER_PARAM, "key1");
79+
LOG_PARAMS_1.putString(EXTERNAL_ARM_VALUE_PARAM, "value1");
80+
LOG_PARAMS_1.putString(EXTERNAL_PERSONALIZATION_ID_PARAM, "p13n1");
81+
LOG_PARAMS_1.putInt(EXTERNAL_ARM_INDEX_PARAM, 0);
82+
LOG_PARAMS_1.putString(EXTERNAL_GROUP_PARAM, "BASELINE");
83+
84+
LOG_PARAMS_2.putString(EXTERNAL_RC_PARAMETER_PARAM, "key2");
85+
LOG_PARAMS_2.putString(EXTERNAL_ARM_VALUE_PARAM, "value2");
86+
LOG_PARAMS_2.putString(EXTERNAL_PERSONALIZATION_ID_PARAM, "p13n2");
87+
LOG_PARAMS_2.putInt(EXTERNAL_ARM_INDEX_PARAM, 1);
88+
LOG_PARAMS_2.putString(EXTERNAL_GROUP_PARAM, "P13N");
89+
90+
INTERNAL_LOG_PARAMS_1.putString(INTERNAL_CHOICE_ID_PARAM, "id1");
91+
92+
INTERNAL_LOG_PARAMS_2.putString(INTERNAL_CHOICE_ID_PARAM, "id2");
7393
}
7494

7595
private static final List<Bundle> FAKE_LOGS = new ArrayList<>();
@@ -89,7 +109,7 @@ public void setUp() {
89109
.when(mockAnalyticsConnector)
90110
.logEvent(
91111
eq(ANALYTICS_ORIGIN_PERSONALIZATION),
92-
or(eq(ANALYTICS_PULL_EVENT), eq(ANALYTICS_PULL_EVENT_INTERNAL)),
112+
or(eq(EXTERNAL_EVENT), eq(INTERNAL_EVENT)),
93113
any(Bundle.class));
94114

95115
personalization = new Personalization(mockAnalyticsConnector);
@@ -104,7 +124,7 @@ public void logArmActive_nonPersonalizationKey_notLogged() {
104124
verify(mockAnalyticsConnector, times(0))
105125
.logEvent(
106126
eq(ANALYTICS_ORIGIN_PERSONALIZATION),
107-
or(eq(ANALYTICS_PULL_EVENT), eq(ANALYTICS_PULL_EVENT_INTERNAL)),
127+
or(eq(EXTERNAL_EVENT), eq(INTERNAL_EVENT)),
108128
any(Bundle.class));
109129
assertThat(FAKE_LOGS).isEmpty();
110130
}
@@ -114,68 +134,35 @@ public void logArmActive_singlePersonalizationKey_loggedInternallyAndExternally(
114134
personalization.logArmActive("key1", CONFIG_CONTAINER);
115135

116136
verify(mockAnalyticsConnector, times(1))
117-
.logEvent(
118-
eq(ANALYTICS_ORIGIN_PERSONALIZATION), eq(ANALYTICS_PULL_EVENT), any(Bundle.class));
137+
.logEvent(eq(ANALYTICS_ORIGIN_PERSONALIZATION), eq(EXTERNAL_EVENT), any(Bundle.class));
119138
verify(mockAnalyticsConnector, times(1))
120-
.logEvent(
121-
eq(ANALYTICS_ORIGIN_PERSONALIZATION),
122-
eq(ANALYTICS_PULL_EVENT_INTERNAL),
123-
any(Bundle.class));
139+
.logEvent(eq(ANALYTICS_ORIGIN_PERSONALIZATION), eq(INTERNAL_EVENT), any(Bundle.class));
124140
assertThat(FAKE_LOGS).hasSize(2);
125141

126-
Bundle logParams = new Bundle();
127-
logParams.putString(ARM_KEY, "key1");
128-
logParams.putString(ARM_VALUE, "value1");
129-
logParams.putString(PERSONALIZATION_ID_LOG_KEY, "p13n1");
130-
logParams.putInt(ARM_INDEX_LOG_KEY, 0);
131-
logParams.putString(GROUP, "BASELINE");
132-
Bundle internalLogParams = new Bundle();
133-
internalLogParams.putString(CHOICE_ID_LOG_KEY, "id1");
134142
assertThat(FAKE_LOGS)
135143
.comparingElementsUsing(TO_STRING)
136-
.containsExactly(logParams.toString(), internalLogParams.toString());
144+
.containsExactly(LOG_PARAMS_1.toString(), INTERNAL_LOG_PARAMS_1.toString());
137145
}
138146

139147
@Test
140-
public void logArmActive_multiplePersonalizationKeys_loggedMultiple() {
148+
public void logArmActive_multiplePersonalizationKeys_loggedInternallyAndExternally() {
141149
personalization.logArmActive("key1", CONFIG_CONTAINER);
142150
personalization.logArmActive("key2", CONFIG_CONTAINER);
143151
personalization.logArmActive("key1", CONFIG_CONTAINER);
144152

145153
verify(mockAnalyticsConnector, times(2))
146-
.logEvent(
147-
eq(ANALYTICS_ORIGIN_PERSONALIZATION), eq(ANALYTICS_PULL_EVENT), any(Bundle.class));
154+
.logEvent(eq(ANALYTICS_ORIGIN_PERSONALIZATION), eq(EXTERNAL_EVENT), any(Bundle.class));
148155
verify(mockAnalyticsConnector, times(2))
149-
.logEvent(
150-
eq(ANALYTICS_ORIGIN_PERSONALIZATION),
151-
eq(ANALYTICS_PULL_EVENT_INTERNAL),
152-
any(Bundle.class));
156+
.logEvent(eq(ANALYTICS_ORIGIN_PERSONALIZATION), eq(INTERNAL_EVENT), any(Bundle.class));
153157
assertThat(FAKE_LOGS).hasSize(4);
154158

155-
Bundle logParams1 = new Bundle();
156-
logParams1.putString(ARM_KEY, "key1");
157-
logParams1.putString(ARM_VALUE, "value1");
158-
logParams1.putString(PERSONALIZATION_ID_LOG_KEY, "p13n1");
159-
logParams1.putInt(ARM_INDEX_LOG_KEY, 0);
160-
logParams1.putString(GROUP, "BASELINE");
161-
Bundle logParams2 = new Bundle();
162-
logParams2.putString(ARM_KEY, "key2");
163-
logParams2.putString(ARM_VALUE, "value2");
164-
logParams2.putString(PERSONALIZATION_ID_LOG_KEY, "p13n2");
165-
logParams2.putInt(ARM_INDEX_LOG_KEY, 1);
166-
logParams2.putString(GROUP, "P13N");
167159
assertThat(FAKE_LOGS)
168160
.comparingElementsUsing(TO_STRING)
169-
.containsAtLeast(logParams1.toString(), logParams2.toString())
161+
.containsAtLeast(LOG_PARAMS_1.toString(), LOG_PARAMS_2.toString())
170162
.inOrder();
171-
172-
Bundle internalLogParams1 = new Bundle();
173-
internalLogParams1.putString(CHOICE_ID_LOG_KEY, "id1");
174-
Bundle internalLogParams2 = new Bundle();
175-
internalLogParams2.putString(CHOICE_ID_LOG_KEY, "id2");
176163
assertThat(FAKE_LOGS)
177164
.comparingElementsUsing(TO_STRING)
178-
.containsAtLeast(internalLogParams1.toString(), internalLogParams2.toString())
165+
.containsAtLeast(INTERNAL_LOG_PARAMS_1.toString(), INTERNAL_LOG_PARAMS_2.toString())
179166
.inOrder();
180167
}
181168
}

0 commit comments

Comments
 (0)