From 9c5082ec44085fb373c04417d1f2e735a56026ea Mon Sep 17 00:00:00 2001 From: Jae Kim Date: Tue, 8 Mar 2022 17:14:27 -0800 Subject: [PATCH 1/5] add builder option for client-engine info --- .../java/com/optimizely/ab/Optimizely.java | 11 +++-- .../ab/event/internal/BuildVersionInfo.java | 13 +++++- .../ab/event/internal/EventFactory.java | 4 +- .../ab/event/internal/payload/EventBatch.java | 4 +- .../optimizely/ab/OptimizelyBuilderTest.java | 43 +++++++++++++++++-- .../ab/event/internal/EventFactoryTest.java | 14 +++--- 6 files changed, 69 insertions(+), 20 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/Optimizely.java b/core-api/src/main/java/com/optimizely/ab/Optimizely.java index 7eae1a1d0..d977ca4fe 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -25,10 +25,7 @@ import com.optimizely.ab.error.ErrorHandler; import com.optimizely.ab.error.NoOpErrorHandler; import com.optimizely.ab.event.*; -import com.optimizely.ab.event.internal.ClientEngineInfo; -import com.optimizely.ab.event.internal.EventFactory; -import com.optimizely.ab.event.internal.UserEvent; -import com.optimizely.ab.event.internal.UserEventFactory; +import com.optimizely.ab.event.internal.*; import com.optimizely.ab.event.internal.payload.EventBatch; import com.optimizely.ab.notification.*; import com.optimizely.ab.optimizelyconfig.OptimizelyConfig; @@ -1519,6 +1516,12 @@ public Builder withUserProfileService(UserProfileService userProfileService) { return this; } + public Builder withClientInfo(EventBatch.ClientEngine clientEngine, String clientVersion) { + ClientEngineInfo.setClientEngine(clientEngine); + BuildVersionInfo.setClientVersion(clientVersion); + return this; + } + @Deprecated public Builder withClientEngine(EventBatch.ClientEngine clientEngine) { logger.info("Deprecated. In the future, set ClientEngine via ClientEngineInfo#setClientEngine."); diff --git a/core-api/src/main/java/com/optimizely/ab/event/internal/BuildVersionInfo.java b/core-api/src/main/java/com/optimizely/ab/event/internal/BuildVersionInfo.java index 3aea4d878..9b060e9a0 100644 --- a/core-api/src/main/java/com/optimizely/ab/event/internal/BuildVersionInfo.java +++ b/core-api/src/main/java/com/optimizely/ab/event/internal/BuildVersionInfo.java @@ -1,6 +1,6 @@ /** * - * Copyright 2016-2017, 2019, Optimizely and contributors + * Copyright 2016-2017, 2019, 2022, Optimizely and contributors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -35,7 +35,16 @@ public final class BuildVersionInfo { private static final Logger logger = LoggerFactory.getLogger(BuildVersionInfo.class); - public final static String VERSION = readVersionNumber(); + public static String VERSION = readVersionNumber(); + + // can be overridden by other wrapper client (android-sdk) + public static void setClientVersion(String version) { + VERSION = version; + } + + public static String getClientVersion() { + return VERSION; + } private static String readVersionNumber() { BufferedReader bufferedReader = null; diff --git a/core-api/src/main/java/com/optimizely/ab/event/internal/EventFactory.java b/core-api/src/main/java/com/optimizely/ab/event/internal/EventFactory.java index 5a881128d..f651be851 100644 --- a/core-api/src/main/java/com/optimizely/ab/event/internal/EventFactory.java +++ b/core-api/src/main/java/com/optimizely/ab/event/internal/EventFactory.java @@ -1,6 +1,6 @@ /** * - * Copyright 2016-2020, Optimizely and contributors + * Copyright 2016-2020, 2022, Optimizely and contributors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -73,7 +73,7 @@ public static LogEvent createLogEvent(List userEvents) { builder .setClientName(ClientEngineInfo.getClientEngine().getClientEngineValue()) - .setClientVersion(BuildVersionInfo.VERSION) + .setClientVersion(BuildVersionInfo.getClientVersion()) .setAccountId(projectConfig.getAccountId()) .setAnonymizeIp(projectConfig.getAnonymizeIP()) .setProjectId(projectConfig.getProjectId()) diff --git a/core-api/src/main/java/com/optimizely/ab/event/internal/payload/EventBatch.java b/core-api/src/main/java/com/optimizely/ab/event/internal/payload/EventBatch.java index fe06b631f..c50ee6288 100644 --- a/core-api/src/main/java/com/optimizely/ab/event/internal/payload/EventBatch.java +++ b/core-api/src/main/java/com/optimizely/ab/event/internal/payload/EventBatch.java @@ -1,6 +1,6 @@ /** * - * Copyright 2018-2019, Optimizely and contributors + * Copyright 2018-2019, 2022, Optimizely and contributors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -165,7 +165,7 @@ public int hashCode() { public static class Builder { private String clientName = ClientEngine.JAVA_SDK.getClientEngineValue(); - private String clientVersion = BuildVersionInfo.VERSION; + private String clientVersion = BuildVersionInfo.getClientVersion(); private String accountId; private List visitors; private Boolean anonymizeIp; diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyBuilderTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyBuilderTest.java index 932150337..e194e63cc 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyBuilderTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyBuilderTest.java @@ -1,6 +1,6 @@ /** * - * Copyright 2016-2017, 2019, Optimizely and contributors + * Copyright 2016-2017, 2019, 2022, Optimizely and contributors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,12 +20,17 @@ import com.optimizely.ab.config.*; import com.optimizely.ab.error.ErrorHandler; import com.optimizely.ab.error.NoOpErrorHandler; +import com.optimizely.ab.event.BatchEventProcessor; import com.optimizely.ab.event.EventHandler; +import com.optimizely.ab.event.LogEvent; +import com.optimizely.ab.event.internal.BuildVersionInfo; +import com.optimizely.ab.event.internal.payload.EventBatch; import com.optimizely.ab.optimizelydecision.OptimizelyDecideOption; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; @@ -34,11 +39,14 @@ import java.util.List; import static com.optimizely.ab.config.DatafileProjectConfigTestUtils.*; +import static junit.framework.Assert.assertEquals; import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.*; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyString; +import static org.mockito.Mockito.*; +import static org.mockito.Mockito.never; /** * Tests for {@link Optimizely#builder(String, EventHandler)}. @@ -195,4 +203,33 @@ public void withDefaultDecideOptions() throws Exception { assertEquals(optimizelyClient.defaultDecideOptions.get(2), OptimizelyDecideOption.EXCLUDE_VARIABLES); } + @Test + public void withClientInfo() throws Exception { + Optimizely optimizely; + EventHandler eventHandler; + ArgumentCaptor argument = ArgumentCaptor.forClass(LogEvent.class); + + // default client-engine info (java-sdk) + + eventHandler = mock(EventHandler.class); + optimizely = Optimizely.builder(validConfigJsonV4(), eventHandler).build(); + optimizely.track("basic_event", "tester"); + + verify(eventHandler, timeout(5000)).dispatchEvent(argument.capture()); + assertEquals(argument.getValue().getEventBatch().getClientName(), "java-sdk"); + assertEquals(argument.getValue().getEventBatch().getClientVersion(), BuildVersionInfo.getClientVersion()); + + // override client-engine info + + eventHandler = mock(EventHandler.class); + optimizely = Optimizely.builder(validConfigJsonV4(), eventHandler) + .withClientInfo(EventBatch.ClientEngine.ANDROID_SDK, "1.2.3") + .build(); + optimizely.track("basic_event", "tester"); + + verify(eventHandler, timeout(5000)).dispatchEvent(argument.capture()); + assertEquals(argument.getValue().getEventBatch().getClientName(), "android-sdk"); + assertEquals(argument.getValue().getEventBatch().getClientVersion(), "1.2.3"); + } + } diff --git a/core-api/src/test/java/com/optimizely/ab/event/internal/EventFactoryTest.java b/core-api/src/test/java/com/optimizely/ab/event/internal/EventFactoryTest.java index 1c5a48313..657bc4fbf 100644 --- a/core-api/src/test/java/com/optimizely/ab/event/internal/EventFactoryTest.java +++ b/core-api/src/test/java/com/optimizely/ab/event/internal/EventFactoryTest.java @@ -1,6 +1,6 @@ /** * - * Copyright 2016-2020, Optimizely and contributors + * Copyright 2016-2020, 2022, Optimizely and contributors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -157,7 +157,7 @@ public void createImpressionEventPassingUserAgentAttribute() throws Exception { assertThat(eventBatch.getAccountId(), is(validProjectConfig.getAccountId())); assertThat(eventBatch.getVisitors().get(0).getAttributes(), is(expectedUserFeatures)); assertThat(eventBatch.getClientName(), is(EventBatch.ClientEngine.JAVA_SDK.getClientEngineValue())); - assertThat(eventBatch.getClientVersion(), is(BuildVersionInfo.VERSION)); + assertThat(eventBatch.getClientVersion(), is(BuildVersionInfo.getClientVersion())); assertNull(eventBatch.getVisitors().get(0).getSessionId()); } @@ -224,7 +224,7 @@ public void createImpressionEvent() throws Exception { assertThat(eventBatch.getAccountId(), is(validProjectConfig.getAccountId())); assertThat(eventBatch.getVisitors().get(0).getAttributes(), is(expectedUserFeatures)); assertThat(eventBatch.getClientName(), is(EventBatch.ClientEngine.JAVA_SDK.getClientEngineValue())); - assertThat(eventBatch.getClientVersion(), is(BuildVersionInfo.VERSION)); + assertThat(eventBatch.getClientVersion(), is(BuildVersionInfo.getClientVersion())); assertNull(eventBatch.getVisitors().get(0).getSessionId()); } @@ -649,7 +649,7 @@ public void createConversionEvent() throws Exception { assertEquals(conversion.getAnonymizeIp(), validProjectConfig.getAnonymizeIP()); assertTrue(conversion.getEnrichDecisions()); assertEquals(conversion.getClientName(), EventBatch.ClientEngine.JAVA_SDK.getClientEngineValue()); - assertEquals(conversion.getClientVersion(), BuildVersionInfo.VERSION); + assertEquals(conversion.getClientVersion(), BuildVersionInfo.getClientVersion()); } /** @@ -717,7 +717,7 @@ public void createConversionEventPassingUserAgentAttribute() throws Exception { assertEquals(conversion.getAnonymizeIp(), validProjectConfig.getAnonymizeIP()); assertTrue(conversion.getEnrichDecisions()); assertEquals(conversion.getClientName(), EventBatch.ClientEngine.JAVA_SDK.getClientEngineValue()); - assertEquals(conversion.getClientVersion(), BuildVersionInfo.VERSION); + assertEquals(conversion.getClientVersion(), BuildVersionInfo.getClientVersion()); } /** @@ -961,7 +961,7 @@ public void createImpressionEventWithBucketingId() throws Exception { assertThat(impression.getVisitors().get(0).getAttributes(), is(expectedUserFeatures)); assertThat(impression.getClientName(), is(EventBatch.ClientEngine.JAVA_SDK.getClientEngineValue())); - assertThat(impression.getClientVersion(), is(BuildVersionInfo.VERSION)); + assertThat(impression.getClientVersion(), is(BuildVersionInfo.getClientVersion())); assertNull(impression.getVisitors().get(0).getSessionId()); } @@ -1034,7 +1034,7 @@ public void createConversionEventWithBucketingId() throws Exception { assertEquals(conversion.getAnonymizeIp(), validProjectConfig.getAnonymizeIP()); assertTrue(conversion.getEnrichDecisions()); assertEquals(conversion.getClientName(), EventBatch.ClientEngine.JAVA_SDK.getClientEngineValue()); - assertEquals(conversion.getClientVersion(), BuildVersionInfo.VERSION); + assertEquals(conversion.getClientVersion(), BuildVersionInfo.getClientVersion()); } From 0098c8ea576578d2d81b189bf9405e347303ee42 Mon Sep 17 00:00:00 2001 From: Jae Kim Date: Wed, 9 Mar 2022 11:39:37 -0800 Subject: [PATCH 2/5] fix spotbugs issues --- .../ab/event/internal/BuildVersionInfo.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/event/internal/BuildVersionInfo.java b/core-api/src/main/java/com/optimizely/ab/event/internal/BuildVersionInfo.java index 9b060e9a0..c709a4da8 100644 --- a/core-api/src/main/java/com/optimizely/ab/event/internal/BuildVersionInfo.java +++ b/core-api/src/main/java/com/optimizely/ab/event/internal/BuildVersionInfo.java @@ -30,20 +30,23 @@ /** * Helper class to retrieve the SDK version information. */ -@Immutable public final class BuildVersionInfo { private static final Logger logger = LoggerFactory.getLogger(BuildVersionInfo.class); - public static String VERSION = readVersionNumber(); + @Deprecated + public final static String VERSION = readVersionNumber(); + + // can be overridden by other wrapper client (android-sdk, etc) + + private static String clientVersion = readVersionNumber(); - // can be overridden by other wrapper client (android-sdk) public static void setClientVersion(String version) { - VERSION = version; + clientVersion = version; } public static String getClientVersion() { - return VERSION; + return clientVersion; } private static String readVersionNumber() { From a854b08b9e228962b98b5f25c3ef11b8ad668f2d Mon Sep 17 00:00:00 2001 From: Jae Kim Date: Mon, 14 Mar 2022 15:42:16 -0700 Subject: [PATCH 3/5] check null client version --- .../com/optimizely/ab/event/internal/BuildVersionInfo.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/core-api/src/main/java/com/optimizely/ab/event/internal/BuildVersionInfo.java b/core-api/src/main/java/com/optimizely/ab/event/internal/BuildVersionInfo.java index c709a4da8..d5620b4e9 100644 --- a/core-api/src/main/java/com/optimizely/ab/event/internal/BuildVersionInfo.java +++ b/core-api/src/main/java/com/optimizely/ab/event/internal/BuildVersionInfo.java @@ -42,7 +42,11 @@ public final class BuildVersionInfo { private static String clientVersion = readVersionNumber(); public static void setClientVersion(String version) { - clientVersion = version; + if (version == null || version.isEmpty()) { + logger.warn("ClientVersion cannot be empty, defaulting to the core java-sdk version."); + return; + } + clientVersion = version; } public static String getClientVersion() { From 2f93f8b3e713e9bfad429d2959862cae545a3b1c Mon Sep 17 00:00:00 2001 From: Jae Kim Date: Mon, 14 Mar 2022 15:59:50 -0700 Subject: [PATCH 4/5] add tests for invalid client name and version --- .../com/optimizely/ab/OptimizelyBuilderTest.java | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyBuilderTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyBuilderTest.java index e194e63cc..91bb19e18 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyBuilderTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyBuilderTest.java @@ -219,9 +219,21 @@ public void withClientInfo() throws Exception { assertEquals(argument.getValue().getEventBatch().getClientName(), "java-sdk"); assertEquals(argument.getValue().getEventBatch().getClientVersion(), BuildVersionInfo.getClientVersion()); + // invalid override with null inputs + + reset(eventHandler); + optimizely = Optimizely.builder(validConfigJsonV4(), eventHandler) + .withClientInfo(null, null) + .build(); + optimizely.track("basic_event", "tester"); + + verify(eventHandler, timeout(5000)).dispatchEvent(argument.capture()); + assertEquals(argument.getValue().getEventBatch().getClientName(), "java-sdk"); + assertEquals(argument.getValue().getEventBatch().getClientVersion(), BuildVersionInfo.getClientVersion()); + // override client-engine info - eventHandler = mock(EventHandler.class); + reset(eventHandler); optimizely = Optimizely.builder(validConfigJsonV4(), eventHandler) .withClientInfo(EventBatch.ClientEngine.ANDROID_SDK, "1.2.3") .build(); From 6abfd6303cfd99b10ced2ba88dc8646de813789a Mon Sep 17 00:00:00 2001 From: Jae Kim Date: Mon, 14 Mar 2022 16:09:44 -0700 Subject: [PATCH 5/5] add doc --- core-api/src/main/java/com/optimizely/ab/Optimizely.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/core-api/src/main/java/com/optimizely/ab/Optimizely.java b/core-api/src/main/java/com/optimizely/ab/Optimizely.java index d977ca4fe..51335ec4f 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -1486,7 +1486,7 @@ public Builder withErrorHandler(ErrorHandler errorHandler) { } /** - * The withEventHandler has has been moved to the EventProcessor which takes a EventHandler in it's builder + * The withEventHandler has been moved to the EventProcessor which takes a EventHandler in it's builder * method. * {@link com.optimizely.ab.event.BatchEventProcessor.Builder#withEventHandler(com.optimizely.ab.event.EventHandler)} label} * Please use that builder method instead. @@ -1516,6 +1516,13 @@ public Builder withUserProfileService(UserProfileService userProfileService) { return this; } + /** + * 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 clientVersion the client SDK version. + * @return An Optimizely builder + */ public Builder withClientInfo(EventBatch.ClientEngine clientEngine, String clientVersion) { ClientEngineInfo.setClientEngine(clientEngine); BuildVersionInfo.setClientVersion(clientVersion);