Skip to content

Commit

Permalink
[FSSDK-9432] fix: fix to support arbitrary client names to be include…
Browse files Browse the repository at this point in the history
…d in logx and odp events. (#524)

Fix to support arbitrary client names to be included in logx and odp events.

- The previous implementation allows only a fixed set of values (java-sdk, android-sdk, and android-tv-sdk) with enum.
- We need to make it more flexible to support other sdks too (flutter, react-native, etc) wrapping the android-sdk and java-sdk cores.
- Old methods for enum access are all deprecated.

Downgrade jackson library version down to 2.13.5 (2.14+ uses Java8 langs not supported in Android)
  • Loading branch information
jaeopt authored Jun 26, 2023
1 parent 3ca11a9 commit c507649
Show file tree
Hide file tree
Showing 10 changed files with 75 additions and 29 deletions.
12 changes: 11 additions & 1 deletion core-api/src/main/java/com/optimizely/ab/Optimizely.java
Original file line number Diff line number Diff line change
Expand Up @@ -1633,10 +1633,20 @@ public Builder withUserProfileService(UserProfileService userProfileService) {
/**
* Override the SDK name and version (for client SDKs like android-sdk wrapping the core java-sdk) to be included in events.
*
* @param clientEngine the client engine type.
* @param clientEngineName the client engine name ("java-sdk", "android-sdk", "flutter-sdk", etc.).
* @param clientVersion the client SDK version.
* @return An Optimizely builder
*/
public Builder withClientInfo(String clientEngineName, String clientVersion) {
ClientEngineInfo.setClientEngineName(clientEngineName);
BuildVersionInfo.setClientVersion(clientVersion);
return this;
}

/**
* @deprecated in favor of {@link withClientInfo(String, String)} which can set with arbitrary client names.
*/
@Deprecated
public Builder withClientInfo(EventBatch.ClientEngine clientEngine, String clientVersion) {
ClientEngineInfo.setClientEngine(clientEngine);
BuildVersionInfo.setClientVersion(clientVersion);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ public final class BuildVersionInfo {
@Deprecated
public final static String VERSION = readVersionNumber();

public final static String DEFAULT_VERSION = readVersionNumber();
// can be overridden by other wrapper client (android-sdk, etc)

private static String clientVersion = readVersionNumber();
private static String clientVersion = DEFAULT_VERSION;

public static void setClientVersion(String version) {
if (version == null || version.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,13 @@
package com.optimizely.ab.event.internal;

import com.optimizely.ab.event.internal.payload.EventBatch;
import com.optimizely.ab.notification.DecisionNotification;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;

/**
* ClientEngineInfo is a utility to globally get and set the ClientEngine used in
* event tracking. The ClientEngine defaults to JAVA_SDK but can be overridden at
Expand All @@ -28,9 +32,34 @@
public class ClientEngineInfo {
private static final Logger logger = LoggerFactory.getLogger(ClientEngineInfo.class);

public static final String DEFAULT_NAME = "java-sdk";
private static String clientEngineName = DEFAULT_NAME;

public static void setClientEngineName(@Nullable String name) {
if (name == null || name.isEmpty()) {
logger.warn("ClientEngineName cannot be empty, defaulting to {}", ClientEngineInfo.clientEngineName);
return;
}
ClientEngineInfo.clientEngineName = name;
}

@Nonnull
public static String getClientEngineName() {
return clientEngineName;
}

private ClientEngineInfo() {
}

@Deprecated
public static final EventBatch.ClientEngine DEFAULT = EventBatch.ClientEngine.JAVA_SDK;
@Deprecated
private static EventBatch.ClientEngine clientEngine = DEFAULT;

/**
* @deprecated in favor of {@link #setClientEngineName(String)} which can set with arbitrary client names.
*/
@Deprecated
public static void setClientEngine(EventBatch.ClientEngine clientEngine) {
if (clientEngine == null) {
logger.warn("ClientEngine cannot be null, defaulting to {}", ClientEngineInfo.clientEngine.getClientEngineValue());
Expand All @@ -39,12 +68,15 @@ public static void setClientEngine(EventBatch.ClientEngine clientEngine) {

logger.info("Setting Optimizely client engine to {}", clientEngine.getClientEngineValue());
ClientEngineInfo.clientEngine = clientEngine;
ClientEngineInfo.clientEngineName = clientEngine.getClientEngineValue();
}

/**
* @deprecated in favor of {@link #getClientEngineName()}.
*/
@Deprecated
public static EventBatch.ClientEngine getClientEngine() {
return clientEngine;
}

private ClientEngineInfo() {
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public static LogEvent createLogEvent(List<UserEvent> userEvents) {
ProjectConfig projectConfig = userContext.getProjectConfig();

builder
.setClientName(ClientEngineInfo.getClientEngine().getClientEngineValue())
.setClientName(ClientEngineInfo.getClientEngineName())
.setClientVersion(BuildVersionInfo.getClientVersion())
.setAccountId(projectConfig.getAccountId())
.setAnonymizeIp(projectConfig.getAnonymizeIP())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
import java.util.List;

public class EventBatch {

@Deprecated
public enum ClientEngine {
JAVA_SDK("java-sdk"),
ANDROID_SDK("android-sdk"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ protected Map<String, Object> augmentCommonData(Map<String, Object> sourceData)
Map<String, Object> data = new HashMap<>();
data.put("idempotence_id", UUID.randomUUID().toString());
data.put("data_source_type", "sdk");
data.put("data_source", ClientEngineInfo.getClientEngine().getClientEngineValue());
data.put("data_source", ClientEngineInfo.getClientEngineName());
data.put("data_source_version", BuildVersionInfo.getClientVersion());

data.putAll(userCommonData);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,6 @@ public void withClientInfo() throws Exception {

reset(eventHandler);
optimizely = Optimizely.builder(validConfigJsonV4(), eventHandler)
.withClientInfo(null, null)
.build();
optimizely.track("basic_event", "tester");

Expand All @@ -245,8 +244,8 @@ public void withClientInfo() throws Exception {
assertEquals(argument.getValue().getEventBatch().getClientVersion(), "1.2.3");

// restore the default values for other tests
ClientEngineInfo.setClientEngine(ClientEngineInfo.DEFAULT);
BuildVersionInfo.setClientVersion(BuildVersionInfo.VERSION);
ClientEngineInfo.setClientEngineName(ClientEngineInfo.DEFAULT_NAME);
BuildVersionInfo.setClientVersion(BuildVersionInfo.DEFAULT_VERSION);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,21 @@ public class ClientEngineInfoTest {

@After
public void tearDown() throws Exception {
ClientEngineInfo.setClientEngine(ClientEngineInfo.DEFAULT);
ClientEngineInfo.setClientEngineName(ClientEngineInfo.DEFAULT_NAME);
}

@Test
public void testSetAndGetClientEngine() {
for (EventBatch.ClientEngine expected: EventBatch.ClientEngine.values()) {
ClientEngineInfo.setClientEngine(expected);
assertEquals(expected, ClientEngineInfo.getClientEngine());
}
}
// default "java-sdk" name
assertEquals("java-sdk", ClientEngineInfo.getClientEngineName());

@Test
public void testDefaultValue() {
assertEquals(ClientEngineInfo.DEFAULT, ClientEngineInfo.getClientEngine());
ClientEngineInfo.setClientEngineName(null);
assertEquals("java-sdk", ClientEngineInfo.getClientEngineName());

ClientEngineInfo.setClientEngineName("");
assertEquals("java-sdk", ClientEngineInfo.getClientEngineName());

ClientEngineInfo.setClientEngineName("test-name");
assertEquals("test-name", ClientEngineInfo.getClientEngineName());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public EventFactoryTest(int datafileVersion,

@After
public void tearDown() {
ClientEngineInfo.setClientEngine(EventBatch.ClientEngine.JAVA_SDK);
ClientEngineInfo.setClientEngineName(ClientEngineInfo.DEFAULT_NAME);
}

/**
Expand Down Expand Up @@ -554,7 +554,7 @@ public void createImpressionEventIgnoresNullAttributes() {
*/
@Test
public void createImpressionEventAndroidClientEngineClientVersion() throws Exception {
ClientEngineInfo.setClientEngine(EventBatch.ClientEngine.ANDROID_SDK);
ClientEngineInfo.setClientEngineName("android-sdk");
ProjectConfig projectConfig = validProjectConfigV2();
Experiment activatedExperiment = projectConfig.getExperiments().get(0);
Variation bucketedVariation = activatedExperiment.getVariations().get(0);
Expand All @@ -566,7 +566,7 @@ public void createImpressionEventAndroidClientEngineClientVersion() throws Excep
userId, attributeMap);
EventBatch impression = gson.fromJson(impressionEvent.getBody(), EventBatch.class);

assertThat(impression.getClientName(), is(EventBatch.ClientEngine.ANDROID_SDK.getClientEngineValue()));
assertThat(impression.getClientName(), is("android-sdk"));
// assertThat(impression.getClientVersion(), is("0.0.0"));
}

Expand All @@ -577,7 +577,7 @@ public void createImpressionEventAndroidClientEngineClientVersion() throws Excep
@Test
public void createImpressionEventAndroidTVClientEngineClientVersion() throws Exception {
String clientVersion = "0.0.0";
ClientEngineInfo.setClientEngine(EventBatch.ClientEngine.ANDROID_TV_SDK);
ClientEngineInfo.setClientEngineName("android-tv-sdk");
ProjectConfig projectConfig = validProjectConfigV2();
Experiment activatedExperiment = projectConfig.getExperiments().get(0);
Variation bucketedVariation = activatedExperiment.getVariations().get(0);
Expand All @@ -589,7 +589,7 @@ public void createImpressionEventAndroidTVClientEngineClientVersion() throws Exc
userId, attributeMap);
EventBatch impression = gson.fromJson(impressionEvent.getBody(), EventBatch.class);

assertThat(impression.getClientName(), is(EventBatch.ClientEngine.ANDROID_TV_SDK.getClientEngineValue()));
assertThat(impression.getClientName(), is("android-tv-sdk"));
// assertThat(impression.getClientVersion(), is(clientVersion));
}

Expand Down Expand Up @@ -816,7 +816,7 @@ public void createConversionEventExperimentStatusPrecedesForcedVariation() {
*/
@Test
public void createConversionEventAndroidClientEngineClientVersion() throws Exception {
ClientEngineInfo.setClientEngine(EventBatch.ClientEngine.ANDROID_SDK);
ClientEngineInfo.setClientEngineName("android-sdk");
Attribute attribute = validProjectConfig.getAttributes().get(0);
EventType eventType = validProjectConfig.getEventTypes().get(0);

Expand All @@ -838,7 +838,7 @@ public void createConversionEventAndroidClientEngineClientVersion() throws Excep

EventBatch conversion = gson.fromJson(conversionEvent.getBody(), EventBatch.class);

assertThat(conversion.getClientName(), is(EventBatch.ClientEngine.ANDROID_SDK.getClientEngineValue()));
assertThat(conversion.getClientName(), is("android-sdk"));
// assertThat(conversion.getClientVersion(), is("0.0.0"));
}

Expand All @@ -849,7 +849,7 @@ public void createConversionEventAndroidClientEngineClientVersion() throws Excep
@Test
public void createConversionEventAndroidTVClientEngineClientVersion() throws Exception {
String clientVersion = "0.0.0";
ClientEngineInfo.setClientEngine(EventBatch.ClientEngine.ANDROID_TV_SDK);
ClientEngineInfo.setClientEngineName("android-tv-sdk");
ProjectConfig projectConfig = validProjectConfigV2();
Attribute attribute = projectConfig.getAttributes().get(0);
EventType eventType = projectConfig.getEventTypes().get(0);
Expand All @@ -873,7 +873,7 @@ public void createConversionEventAndroidTVClientEngineClientVersion() throws Exc

EventBatch conversion = gson.fromJson(conversionEvent.getBody(), EventBatch.class);

assertThat(conversion.getClientName(), is(EventBatch.ClientEngine.ANDROID_TV_SDK.getClientEngineValue()));
assertThat(conversion.getClientName(), is("android-tv-sdk"));
// assertThat(conversion.getClientVersion(), is(clientVersion));
}

Expand Down
3 changes: 2 additions & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ org.gradle.parallel = true
gsonVersion = 2.10.1
guavaVersion = 22.0
hamcrestVersion = 1.3
jacksonVersion = 2.15.2
# NOTE: jackson 2.14+ uses Java8 stream apis not supported in android
jacksonVersion = 2.13.5
jsonVersion = 20190722
jsonSimpleVersion = 1.1.1
logbackVersion = 1.2.3
Expand Down

0 comments on commit c507649

Please sign in to comment.