From e2f9074986d4a884a4a31a3e5bded38764131035 Mon Sep 17 00:00:00 2001 From: Aaron Silverman Date: Mon, 12 Aug 2024 22:38:10 -0600 Subject: [PATCH 1/7] work in progress --- build.gradle | 6 + src/main/java/cloud/eppo/BaseEppoClient.java | 12 +- .../java/cloud/eppo/BaseEppoClientTest.java | 143 +---------------- .../eppo/helpers/AssignmentTestCase.java | 150 ++++++++++++++++++ 4 files changed, 162 insertions(+), 149 deletions(-) diff --git a/build.gradle b/build.gradle index 35613f76..047a8059 100644 --- a/build.gradle +++ b/build.gradle @@ -62,11 +62,17 @@ java { withSourcesJar() } +task testJar(type: Jar) { + archiveClassifier.set('tests') + from sourceSets.test.output +} + publishing { publications { mavenJava(MavenPublication) { artifactId = 'sdk-common-jvm' from components.java + artifact testJar // Include the test-jar in the published artifacts versionMapping { usage('java-api') { fromResolutionOf('runtimeClasspath') diff --git a/src/main/java/cloud/eppo/BaseEppoClient.java b/src/main/java/cloud/eppo/BaseEppoClient.java index 295ca91e..04584edb 100644 --- a/src/main/java/cloud/eppo/BaseEppoClient.java +++ b/src/main/java/cloud/eppo/BaseEppoClient.java @@ -41,7 +41,7 @@ public class BaseEppoClient { /** @noinspection FieldMayBeFinal */ private static EppoHttpClient httpClientOverride = null; - private BaseEppoClient( + protected BaseEppoClient( String apiKey, String sdkName, String sdkVersion, @@ -78,7 +78,7 @@ private EppoHttpClient buildHttpClient( return httpClient; } - public static BaseEppoClient init( + protected static BaseEppoClient init( String apiKey, String sdkName, String sdkVersion, @@ -114,14 +114,6 @@ public static BaseEppoClient init( return instance; } - /** - * Ability to ad-hoc kick off a configuration load. Will load from a filesystem cached file as - * well as fire off an HTTPS request for an updated configuration. If the cache load finishes - * first, those assignments will be used until the fetch completes. - * - *

Deprecated, as we plan to make a more targeted and configurable way to do so in the future. - */ - @Deprecated public void refreshConfiguration() { requestor.load(); } diff --git a/src/test/java/cloud/eppo/BaseEppoClientTest.java b/src/test/java/cloud/eppo/BaseEppoClientTest.java index 9b047cc0..102bebd6 100644 --- a/src/test/java/cloud/eppo/BaseEppoClientTest.java +++ b/src/test/java/cloud/eppo/BaseEppoClientTest.java @@ -1,35 +1,28 @@ package cloud.eppo; +import static cloud.eppo.helpers.AssignmentTestCase.parseTestCaseFile; +import static cloud.eppo.helpers.AssignmentTestCase.runTestCase; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.*; import cloud.eppo.helpers.AssignmentTestCase; -import cloud.eppo.helpers.AssignmentTestCaseDeserializer; -import cloud.eppo.helpers.SubjectAssignment; -import cloud.eppo.helpers.TestCaseValue; import cloud.eppo.logging.Assignment; import cloud.eppo.logging.AssignmentLogger; import cloud.eppo.ufc.dto.Attributes; import cloud.eppo.ufc.dto.EppoValue; import cloud.eppo.ufc.dto.VariationType; import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.module.SimpleModule; import java.io.File; -import java.io.IOException; import java.lang.reflect.Field; import java.util.*; import java.util.stream.Stream; import okhttp3.*; -import org.apache.commons.io.FileUtils; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -44,7 +37,7 @@ public class BaseEppoClientTest { private static final String DUMMY_FLAG_API_KEY = "dummy-flags-api-key"; // Will load flags-v1 private static final String TEST_HOST = "https://us-central1-eppo-qa.cloudfunctions.net/serveGitHubRacTestFile"; - private final ObjectMapper mapper = new ObjectMapper().registerModule(module()); + private final ObjectMapper mapper = new ObjectMapper().registerModule(AssignmentTestCase.assignmentTestCaseModule()); private AssignmentLogger mockAssignmentLogger; @@ -93,129 +86,7 @@ public void testObfuscatedAssignments(File testFile) { } private static Stream getAssignmentTestData() { - File testCaseFolder = new File("src/test/resources/shared/ufc/tests"); - File[] testCaseFiles = testCaseFolder.listFiles(); - assertNotNull(testCaseFiles); - assertTrue(testCaseFiles.length > 0); - List arguments = new ArrayList<>(); - for (File testCaseFile : testCaseFiles) { - arguments.add(Arguments.of(testCaseFile)); - } - return arguments.stream(); - } - - private AssignmentTestCase parseTestCaseFile(File testCaseFile) { - AssignmentTestCase testCase; - try { - String json = FileUtils.readFileToString(testCaseFile, "UTF8"); - testCase = mapper.readValue(json, AssignmentTestCase.class); - } catch (IOException ex) { - throw new RuntimeException(ex); - } - return testCase; - } - - private void runTestCase(AssignmentTestCase testCase) { - String flagKey = testCase.getFlag(); - TestCaseValue defaultValue = testCase.getDefaultValue(); - BaseEppoClient eppoClient = BaseEppoClient.getInstance(); - assertFalse(testCase.getSubjects().isEmpty()); - - for (SubjectAssignment subjectAssignment : testCase.getSubjects()) { - String subjectKey = subjectAssignment.getSubjectKey(); - Attributes subjectAttributes = subjectAssignment.getSubjectAttributes(); - - // Depending on the variation type, we will need to change which assignment method we call and - // how we get the default value - switch (testCase.getVariationType()) { - case BOOLEAN: - boolean boolAssignment = - eppoClient.getBooleanAssignment( - flagKey, subjectKey, subjectAttributes, defaultValue.booleanValue()); - assertAssignment(flagKey, subjectAssignment, boolAssignment); - break; - case INTEGER: - int intAssignment = - eppoClient.getIntegerAssignment( - flagKey, - subjectKey, - subjectAttributes, - Double.valueOf(defaultValue.doubleValue()).intValue()); - assertAssignment(flagKey, subjectAssignment, intAssignment); - break; - case NUMERIC: - double doubleAssignment = - eppoClient.getDoubleAssignment( - flagKey, subjectKey, subjectAttributes, defaultValue.doubleValue()); - assertAssignment(flagKey, subjectAssignment, doubleAssignment); - break; - case STRING: - String stringAssignment = - eppoClient.getStringAssignment( - flagKey, subjectKey, subjectAttributes, defaultValue.stringValue()); - assertAssignment(flagKey, subjectAssignment, stringAssignment); - break; - case JSON: - JsonNode jsonAssignment = - eppoClient.getJSONAssignment( - flagKey, subjectKey, subjectAttributes, testCase.getDefaultValue().jsonValue()); - assertAssignment(flagKey, subjectAssignment, jsonAssignment); - break; - default: - throw new UnsupportedOperationException( - "Unexpected variation type " - + testCase.getVariationType() - + " for " - + flagKey - + " test case"); - } - } - } - - /** Helper method for asserting a subject assignment with a useful failure message. */ - private void assertAssignment( - String flagKey, SubjectAssignment expectedSubjectAssignment, T assignment) { - - if (assignment == null) { - fail( - "Unexpected null " - + flagKey - + " assignment for subject " - + expectedSubjectAssignment.getSubjectKey()); - } - - String failureMessage = - "Incorrect " - + flagKey - + " assignment for subject " - + expectedSubjectAssignment.getSubjectKey(); - - if (assignment instanceof Boolean) { - assertEquals( - expectedSubjectAssignment.getAssignment().booleanValue(), assignment, failureMessage); - } else if (assignment instanceof Integer) { - assertEquals( - Double.valueOf(expectedSubjectAssignment.getAssignment().doubleValue()).intValue(), - assignment, - failureMessage); - } else if (assignment instanceof Double) { - assertEquals( - expectedSubjectAssignment.getAssignment().doubleValue(), - (Double) assignment, - 0.000001, - failureMessage); - } else if (assignment instanceof String) { - assertEquals( - expectedSubjectAssignment.getAssignment().stringValue(), assignment, failureMessage); - } else if (assignment instanceof JsonNode) { - assertEquals( - expectedSubjectAssignment.getAssignment().jsonValue().toString(), - assignment.toString(), - failureMessage); - } else { - throw new IllegalArgumentException( - "Unexpected assignment type " + assignment.getClass().getCanonicalName()); - } + return AssignmentTestCase.getAssignmentTestData(); } @Test @@ -440,11 +311,5 @@ private void setOverrideField(String fieldName, T override) { } } - private static SimpleModule module() { - SimpleModule module = new SimpleModule(); - module.addDeserializer(AssignmentTestCase.class, new AssignmentTestCaseDeserializer()); - return module; - } - // TODO: tests for the cache } diff --git a/src/test/java/cloud/eppo/helpers/AssignmentTestCase.java b/src/test/java/cloud/eppo/helpers/AssignmentTestCase.java index 7c2e9ea0..d1857c2d 100644 --- a/src/test/java/cloud/eppo/helpers/AssignmentTestCase.java +++ b/src/test/java/cloud/eppo/helpers/AssignmentTestCase.java @@ -1,7 +1,22 @@ package cloud.eppo.helpers; +import cloud.eppo.BaseEppoClient; +import cloud.eppo.ufc.dto.Attributes; import cloud.eppo.ufc.dto.VariationType; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.module.SimpleModule; +import org.apache.commons.io.FileUtils; +import org.junit.jupiter.params.provider.Arguments; + +import java.io.File; +import java.io.IOException; +import java.util.ArrayList; import java.util.List; +import java.util.stream.Stream; + +import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertEquals; public class AssignmentTestCase { private final String flag; @@ -44,4 +59,139 @@ public void setFileName(String fileName) { public String getFileName() { return fileName; } + + private static ObjectMapper mapper = new ObjectMapper().registerModule(assignmentTestCaseModule()); + + public static SimpleModule assignmentTestCaseModule() { + SimpleModule module = new SimpleModule(); + module.addDeserializer(AssignmentTestCase.class, new AssignmentTestCaseDeserializer()); + return module; + } + + public static Stream getAssignmentTestData() { + File testCaseFolder = new File("src/test/resources/shared/ufc/tests"); + File[] testCaseFiles = testCaseFolder.listFiles(); + assertNotNull(testCaseFiles); + assertTrue(testCaseFiles.length > 0); + List arguments = new ArrayList<>(); + for (File testCaseFile : testCaseFiles) { + arguments.add(Arguments.of(testCaseFile)); + } + return arguments.stream(); + } + + public static AssignmentTestCase parseTestCaseFile(File testCaseFile) { + AssignmentTestCase testCase; + try { + String json = FileUtils.readFileToString(testCaseFile, "UTF8"); + + testCase = mapper.readValue(json, AssignmentTestCase.class); + } catch (IOException ex) { + throw new RuntimeException(ex); + } + return testCase; + } + + public static void runTestCase(AssignmentTestCase testCase) { + String flagKey = testCase.getFlag(); + TestCaseValue defaultValue = testCase.getDefaultValue(); + BaseEppoClient eppoClient = BaseEppoClient.getInstance(); + assertFalse(testCase.getSubjects().isEmpty()); + + for (SubjectAssignment subjectAssignment : testCase.getSubjects()) { + String subjectKey = subjectAssignment.getSubjectKey(); + Attributes subjectAttributes = subjectAssignment.getSubjectAttributes(); + + // Depending on the variation type, we will need to change which assignment method we call and + // how we get the default value + switch (testCase.getVariationType()) { + case BOOLEAN: + boolean boolAssignment = + eppoClient.getBooleanAssignment( + flagKey, subjectKey, subjectAttributes, defaultValue.booleanValue()); + assertAssignment(flagKey, subjectAssignment, boolAssignment); + break; + case INTEGER: + int intAssignment = + eppoClient.getIntegerAssignment( + flagKey, + subjectKey, + subjectAttributes, + Double.valueOf(defaultValue.doubleValue()).intValue()); + assertAssignment(flagKey, subjectAssignment, intAssignment); + break; + case NUMERIC: + double doubleAssignment = + eppoClient.getDoubleAssignment( + flagKey, subjectKey, subjectAttributes, defaultValue.doubleValue()); + assertAssignment(flagKey, subjectAssignment, doubleAssignment); + break; + case STRING: + String stringAssignment = + eppoClient.getStringAssignment( + flagKey, subjectKey, subjectAttributes, defaultValue.stringValue()); + assertAssignment(flagKey, subjectAssignment, stringAssignment); + break; + case JSON: + JsonNode jsonAssignment = + eppoClient.getJSONAssignment( + flagKey, subjectKey, subjectAttributes, testCase.getDefaultValue().jsonValue()); + assertAssignment(flagKey, subjectAssignment, jsonAssignment); + break; + default: + throw new UnsupportedOperationException( + "Unexpected variation type " + + testCase.getVariationType() + + " for " + + flagKey + + " test case"); + } + } + } + + /** Helper method for asserting a subject assignment with a useful failure message. */ + public static void assertAssignment( + String flagKey, SubjectAssignment expectedSubjectAssignment, T assignment) { + + if (assignment == null) { + fail( + "Unexpected null " + + flagKey + + " assignment for subject " + + expectedSubjectAssignment.getSubjectKey()); + } + + String failureMessage = + "Incorrect " + + flagKey + + " assignment for subject " + + expectedSubjectAssignment.getSubjectKey(); + + if (assignment instanceof Boolean) { + assertEquals( + expectedSubjectAssignment.getAssignment().booleanValue(), assignment, failureMessage); + } else if (assignment instanceof Integer) { + assertEquals( + Double.valueOf(expectedSubjectAssignment.getAssignment().doubleValue()).intValue(), + assignment, + failureMessage); + } else if (assignment instanceof Double) { + assertEquals( + expectedSubjectAssignment.getAssignment().doubleValue(), + (Double) assignment, + 0.000001, + failureMessage); + } else if (assignment instanceof String) { + assertEquals( + expectedSubjectAssignment.getAssignment().stringValue(), assignment, failureMessage); + } else if (assignment instanceof JsonNode) { + assertEquals( + expectedSubjectAssignment.getAssignment().jsonValue().toString(), + assignment.toString(), + failureMessage); + } else { + throw new IllegalArgumentException( + "Unexpected assignment type " + assignment.getClass().getCanonicalName()); + } + } } From e7880d5c7a90e981dfa243f45f430ac081efc854 Mon Sep 17 00:00:00 2001 From: Aaron Silverman Date: Tue, 13 Aug 2024 10:17:21 -0600 Subject: [PATCH 2/7] remove singleton for base client --- build.gradle | 6 +- src/main/java/cloud/eppo/BaseEppoClient.java | 129 ++++-------------- src/main/java/cloud/eppo/EppoHttpClient.java | 25 +--- .../eppo/EppoHttpClientRequestCallback.java | 6 + .../eppo/ufc/dto/DiscriminableAttributes.java | 6 +- .../cloud/eppo/BaseEppoClientBanditTest.java | 45 +++--- .../java/cloud/eppo/BaseEppoClientTest.java | 64 ++++----- .../java/cloud/eppo/FlagEvaluatorTest.java | 1 - .../eppo/helpers/AssignmentTestCase.java | 6 +- 9 files changed, 102 insertions(+), 186 deletions(-) create mode 100644 src/main/java/cloud/eppo/EppoHttpClientRequestCallback.java diff --git a/build.gradle b/build.gradle index 047a8059..ded52ad4 100644 --- a/build.gradle +++ b/build.gradle @@ -62,7 +62,7 @@ java { withSourcesJar() } -task testJar(type: Jar) { +tasks.register('testJar', Jar) { archiveClassifier.set('tests') from sourceSets.test.output } @@ -120,7 +120,7 @@ publishing { // Custom task to ensure we can conditionally publish either a release or snapshot artifact // based on a command line switch. See github workflow files for more details on usage. -task checkVersion { +tasks.register('checkVersion') { doLast { if (!project.hasProperty('release') && !project.hasProperty('snapshot')) { throw new GradleException("You must specify either -Prelease or -Psnapshot") @@ -141,7 +141,7 @@ tasks.named('publish').configure { } // Conditionally enable or disable publishing tasks -tasks.withType(PublishToMavenRepository) { +tasks.withType(PublishToMavenRepository).configureEach { onlyIf { project.ext.has('shouldPublish') && project.ext.shouldPublish } diff --git a/src/main/java/cloud/eppo/BaseEppoClient.java b/src/main/java/cloud/eppo/BaseEppoClient.java index 04584edb..fc9ef7ed 100644 --- a/src/main/java/cloud/eppo/BaseEppoClient.java +++ b/src/main/java/cloud/eppo/BaseEppoClient.java @@ -22,11 +22,12 @@ public class BaseEppoClient { private final ObjectMapper mapper = new ObjectMapper() .registerModule(EppoModule.eppoModule()); // TODO: is this the best place for this? - private static final String DEFAULT_HOST = "https://fscdn.eppo.cloud"; - private static final boolean DEFAULT_IS_GRACEFUL_MODE = true; + + protected static final String DEFAULT_HOST = "https://fscdn.eppo.cloud"; + protected static final boolean DEFAULT_IS_GRACEFUL_MODE = true; + protected final ConfigurationRequestor requestor; private final ConfigurationStore configurationStore; - private final ConfigurationRequestor requestor; private final AssignmentLogger assignmentLogger; private final BanditLogger banditLogger; private final String sdkName; @@ -34,25 +35,35 @@ public class BaseEppoClient { private final boolean isConfigObfuscated; private boolean isGracefulMode; - private static BaseEppoClient instance; - // Fields useful for testing in situations where we want to mock the http client or configuration // store (accessed via reflection) /** @noinspection FieldMayBeFinal */ private static EppoHttpClient httpClientOverride = null; - protected BaseEppoClient( + public BaseEppoClient( String apiKey, String sdkName, String sdkVersion, String host, - ConfigurationStore configurationStore, AssignmentLogger assignmentLogger, BanditLogger banditLogger, - boolean isGracefulMode) { + boolean isGracefulMode, + boolean expectObfuscatedConfig + ) { + + if (apiKey == null) { + throw new IllegalArgumentException("Unable to initialize Eppo SDK due to missing API key"); + } + if (sdkName == null || sdkVersion == null) { + throw new IllegalArgumentException( + "Unable to initialize Eppo SDK due to missing SDK name or version"); + } + if (host == null) { + host = DEFAULT_HOST; + } EppoHttpClient httpClient = buildHttpClient(host, apiKey, sdkName, sdkVersion); - this.configurationStore = configurationStore; + this.configurationStore = new ConfigurationStore(); requestor = new ConfigurationRequestor(configurationStore, httpClient); this.assignmentLogger = assignmentLogger; this.banditLogger = banditLogger; @@ -61,8 +72,10 @@ protected BaseEppoClient( this.sdkName = sdkName; this.sdkVersion = sdkVersion; // For now, the configuration is only obfuscated for Android clients - this.isConfigObfuscated = sdkName.toLowerCase().contains("android"); + this.isConfigObfuscated = expectObfuscatedConfig; + // TODO: caching initialization (such as setting an API-key-specific prefix + // will probably involve passing in configurationStore to the constructor } private EppoHttpClient buildHttpClient( @@ -78,43 +91,7 @@ private EppoHttpClient buildHttpClient( return httpClient; } - protected static BaseEppoClient init( - String apiKey, - String sdkName, - String sdkVersion, - String host, - AssignmentLogger assignmentLogger, - BanditLogger banditLogger, - boolean isGracefulMode) { - - if (apiKey == null) { - throw new IllegalArgumentException("Unable to initialize Eppo SDK due to missing API key"); - } - if (sdkName == null || sdkVersion == null) { - throw new IllegalArgumentException( - "Unable to initialize Eppo SDK due to missing SDK name or version"); - } - - if (instance != null) { - // TODO: also check we're not running a test - log.warn("Reinitializing an Eppo Client instance that was already initialized"); - } - instance = - new BaseEppoClient( - apiKey, - sdkName, - sdkVersion, - host, - new ConfigurationStore(), - assignmentLogger, - banditLogger, - isGracefulMode); - instance.refreshConfiguration(); - - return instance; - } - - public void refreshConfiguration() { + protected void loadConfiguration() { requestor.load(); } @@ -471,65 +448,7 @@ private T throwIfNotGraceful(Exception e, T defaultValue) { throw new RuntimeException(e); } - public static BaseEppoClient getInstance() { - if (BaseEppoClient.instance == null) { - throw new IllegalStateException("Eppo SDK has not been initialized"); - } - - return BaseEppoClient.instance; - } - public void setIsGracefulFailureMode(boolean isGracefulFailureMode) { this.isGracefulMode = isGracefulFailureMode; } - - public static class Builder { - private String apiKey; - private String sdkName; - private String sdkVersion; - private String host = DEFAULT_HOST; - private AssignmentLogger assignmentLogger; - private BanditLogger banditLogger; - private boolean isGracefulMode = DEFAULT_IS_GRACEFUL_MODE; - - public Builder apiKey(String apiKey) { - this.apiKey = apiKey; - return this; - } - - public Builder sdkName(String sdkName) { - this.sdkName = sdkName; - return this; - } - - public Builder sdkVersion(String sdkVersion) { - this.sdkVersion = sdkVersion; - return this; - } - - public Builder host(String host) { - this.host = host; - return this; - } - - public Builder assignmentLogger(AssignmentLogger assignmentLogger) { - this.assignmentLogger = assignmentLogger; - return this; - } - - public Builder banditLogger(BanditLogger banditLogger) { - this.banditLogger = banditLogger; - return this; - } - - public Builder isGracefulMode(boolean isGracefulMode) { - this.isGracefulMode = isGracefulMode; - return this; - } - - public BaseEppoClient buildAndInit() { - return BaseEppoClient.init( - apiKey, sdkName, sdkVersion, host, assignmentLogger, banditLogger, isGracefulMode); - } - } } diff --git a/src/main/java/cloud/eppo/EppoHttpClient.java b/src/main/java/cloud/eppo/EppoHttpClient.java index 6daf2fe7..8cc81daa 100644 --- a/src/main/java/cloud/eppo/EppoHttpClient.java +++ b/src/main/java/cloud/eppo/EppoHttpClient.java @@ -59,7 +59,7 @@ public Response get(String path) { } } - public void get(String path, RequestCallback callback) { + public void get(String path, EppoHttpClientRequestCallback callback) { HttpUrl httpUrl = HttpUrl.parse(baseUrl + path) .newBuilder() @@ -83,13 +83,11 @@ public void onResponse(Call call, Response response) { callback.onFailure("Failed to read response from URL " + httpUrl); } } else { - switch (response.code()) { - case HttpURLConnection.HTTP_FORBIDDEN: - callback.onFailure("Invalid API key"); - break; - default: - log.debug("Fetch failed with status code: " + response.code()); - callback.onFailure("Bad response from URL " + httpUrl); + if (response.code() == HttpURLConnection.HTTP_FORBIDDEN) { + callback.onFailure("Invalid API key"); + } else { + log.debug("Fetch failed with status code: {}", response.code()); + callback.onFailure("Bad response from URL " + httpUrl); } } response.close(); @@ -97,20 +95,11 @@ public void onResponse(Call call, Response response) { @Override public void onFailure(Call call, IOException e) { - log.error( - "Http request failure: " - + e.getMessage() - + " " - + Arrays.toString(e.getStackTrace()), - e); + log.error("Http request failure: {} {}", e.getMessage(), Arrays.toString(e.getStackTrace()), e); callback.onFailure("Unable to fetch from URL " + httpUrl); } }); } } -interface RequestCallback { - void onSuccess(String responseBody); - void onFailure(String errorMessage); -} diff --git a/src/main/java/cloud/eppo/EppoHttpClientRequestCallback.java b/src/main/java/cloud/eppo/EppoHttpClientRequestCallback.java new file mode 100644 index 00000000..9258219b --- /dev/null +++ b/src/main/java/cloud/eppo/EppoHttpClientRequestCallback.java @@ -0,0 +1,6 @@ +package cloud.eppo; + +public interface EppoHttpClientRequestCallback { + void onSuccess(String responseBody); + void onFailure(String errorMessage); +} diff --git a/src/main/java/cloud/eppo/ufc/dto/DiscriminableAttributes.java b/src/main/java/cloud/eppo/ufc/dto/DiscriminableAttributes.java index 48d9432c..80b5ee3e 100644 --- a/src/main/java/cloud/eppo/ufc/dto/DiscriminableAttributes.java +++ b/src/main/java/cloud/eppo/ufc/dto/DiscriminableAttributes.java @@ -2,9 +2,9 @@ public interface DiscriminableAttributes { - public Attributes getNumericAttributes(); + Attributes getNumericAttributes(); - public Attributes getCategoricalAttributes(); + Attributes getCategoricalAttributes(); - public Attributes getAllAttributes(); + Attributes getAllAttributes(); } diff --git a/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java b/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java index b1463419..86cd45d4 100644 --- a/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java +++ b/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java @@ -39,20 +39,24 @@ public class BaseEppoClientBanditTest { private static final BanditLogger mockBanditLogger = mock(BanditLogger.class); private static final Date testStart = new Date(); + private static BaseEppoClient eppoClient; + // TODO: possibly consolidate code between this and the non-bandit test @BeforeAll public static void initClient() { - - new BaseEppoClient.Builder() - .apiKey(DUMMY_BANDIT_API_KEY) - .sdkName("java") - .sdkVersion("3.0.0") - .isGracefulMode(false) - .host(TEST_HOST) - .assignmentLogger(mockAssignmentLogger) - .banditLogger(mockBanditLogger) - .buildAndInit(); + eppoClient = new BaseEppoClient( + DUMMY_BANDIT_API_KEY, + "java", + "3.0.0", + TEST_HOST, + mockAssignmentLogger, + mockBanditLogger, + false, + false + ); + + eppoClient.loadConfiguration(); log.info("Test client initialized"); } @@ -62,7 +66,7 @@ public void reset() { clearInvocations(mockAssignmentLogger); clearInvocations(mockBanditLogger); doNothing().when(mockBanditLogger).logBanditAssignment(any()); - BaseEppoClient.getInstance().setIsGracefulFailureMode(false); + eppoClient.setIsGracefulFailureMode(false); } @ParameterizedTest @@ -106,7 +110,7 @@ private void runBanditTestCase(BanditTestCase testCase) { ContextAttributes attributes = subjectAssignment.getSubjectAttributes(); Actions actions = subjectAssignment.getActions(); BanditResult assignment = - BaseEppoClient.getInstance() + eppoClient .getBanditAction(flagKey, subjectKey, attributes, actions, defaultValue); assertBanditAssignment(flagKey, subjectAssignment, assignment); } @@ -138,6 +142,7 @@ private void assertBanditAssignment( failureMessage); } + @SuppressWarnings("ExtractMethodRecommender") @Test public void testBanditLogsAction() { String flagKey = "banner_bandit_flag"; @@ -165,7 +170,7 @@ public void testBanditLogsAction() { actions.put("reebok", rebookAttributes); BanditResult banditResult = - BaseEppoClient.getInstance() + eppoClient .getBanditAction(flagKey, subjectKey, subjectAttributes, actions, "control"); // Verify assignment @@ -239,7 +244,7 @@ public void testNoBanditLogsWhenNotBandit() { actions.put("adidas", new Attributes()); BanditResult banditResult = - BaseEppoClient.getInstance() + eppoClient .getBanditAction(flagKey, subjectKey, subjectAttributes, actions, "default"); // Verify assignment @@ -267,7 +272,7 @@ public void testNoBanditLogsWhenNoActions() { BanditActions actions = new BanditActions(); BanditResult banditResult = - BaseEppoClient.getInstance() + eppoClient .getBanditAction(flagKey, subjectKey, subjectAttributes, actions, "control"); // Verify assignment @@ -286,7 +291,7 @@ public void testNoBanditLogsWhenNoActions() { @Test public void testBanditErrorGracefulModeOff() { - BaseEppoClient.getInstance() + eppoClient .setIsGracefulFailureMode( false); // Should be set by @BeforeEach but repeated here for test clarity try (MockedStatic mockedStatic = mockStatic(BanditEvaluator.class)) { @@ -302,7 +307,7 @@ public void testBanditErrorGracefulModeOff() { assertThrows( RuntimeException.class, () -> - BaseEppoClient.getInstance() + eppoClient .getBanditAction( "banner_bandit_flag", "subject", new Attributes(), actions, "default")); } @@ -310,7 +315,7 @@ public void testBanditErrorGracefulModeOff() { @Test public void testBanditErrorGracefulModeOn() { - BaseEppoClient.getInstance().setIsGracefulFailureMode(true); + eppoClient.setIsGracefulFailureMode(true); try (MockedStatic mockedStatic = mockStatic(BanditEvaluator.class)) { // Configure the mock to throw an exception mockedStatic @@ -322,7 +327,7 @@ public void testBanditErrorGracefulModeOn() { actions.put("nike", new Attributes()); actions.put("adidas", new Attributes()); BanditResult banditResult = - BaseEppoClient.getInstance() + eppoClient .getBanditAction( "banner_bandit_flag", "subject", new Attributes(), actions, "default"); assertEquals("banner_bandit", banditResult.getVariation()); @@ -341,7 +346,7 @@ public void testBanditLogErrorNonFatal() { actions.put("nike", new Attributes()); actions.put("adidas", new Attributes()); BanditResult banditResult = - BaseEppoClient.getInstance() + eppoClient .getBanditAction("banner_bandit_flag", "subject", new Attributes(), actions, "default"); assertEquals("banner_bandit", banditResult.getVariation()); assertEquals("nike", banditResult.getAction()); diff --git a/src/test/java/cloud/eppo/BaseEppoClientTest.java b/src/test/java/cloud/eppo/BaseEppoClientTest.java index 102bebd6..cf290273 100644 --- a/src/test/java/cloud/eppo/BaseEppoClientTest.java +++ b/src/test/java/cloud/eppo/BaseEppoClientTest.java @@ -6,8 +6,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyString; + import static org.mockito.Mockito.*; import cloud.eppo.helpers.AssignmentTestCase; @@ -39,27 +38,30 @@ public class BaseEppoClientTest { "https://us-central1-eppo-qa.cloudfunctions.net/serveGitHubRacTestFile"; private final ObjectMapper mapper = new ObjectMapper().registerModule(AssignmentTestCase.assignmentTestCaseModule()); + private BaseEppoClient eppoClient; private AssignmentLogger mockAssignmentLogger; // TODO: async init client tests private void initClient() { - initClient(TEST_HOST, false, false, DUMMY_FLAG_API_KEY); + initClient(false, false); } - private void initClient( - String host, boolean isGracefulMode, boolean isConfigObfuscated, String apiKey) { + private void initClient(boolean isGracefulMode, boolean isConfigObfuscated) { mockAssignmentLogger = mock(AssignmentLogger.class); - new BaseEppoClient.Builder() - .apiKey(apiKey) - .sdkName(isConfigObfuscated ? "android" : "java") - .sdkVersion("3.0.0") - .isGracefulMode(isGracefulMode) - .host(host) - .assignmentLogger(mockAssignmentLogger) - .buildAndInit(); - + eppoClient = new BaseEppoClient( + DUMMY_FLAG_API_KEY, + isConfigObfuscated ? "android" : "java", + "3.0.0", + TEST_HOST, + mockAssignmentLogger, + null, + isGracefulMode, + isConfigObfuscated + ); + + eppoClient.loadConfiguration(); log.info("Test client initialized"); } @@ -72,17 +74,17 @@ public void cleanUp() { @ParameterizedTest @MethodSource("getAssignmentTestData") public void testUnobfuscatedAssignments(File testFile) { - initClient(TEST_HOST, false, false, DUMMY_FLAG_API_KEY); + initClient(false, false); AssignmentTestCase testCase = parseTestCaseFile(testFile); - runTestCase(testCase); + runTestCase(testCase, eppoClient); } @ParameterizedTest @MethodSource("getAssignmentTestData") public void testObfuscatedAssignments(File testFile) { - initClient(TEST_HOST, false, true, DUMMY_FLAG_API_KEY); + initClient(false, true); AssignmentTestCase testCase = parseTestCaseFile(testFile); - runTestCase(testCase); + runTestCase(testCase, eppoClient); } private static Stream getAssignmentTestData() { @@ -91,9 +93,9 @@ private static Stream getAssignmentTestData() { @Test public void testErrorGracefulModeOn() throws JsonProcessingException { - initClient(TEST_HOST, true, false, DUMMY_FLAG_API_KEY); + initClient(true, false); - BaseEppoClient realClient = BaseEppoClient.getInstance(); + BaseEppoClient realClient = eppoClient; BaseEppoClient spyClient = spy(realClient); doThrow(new RuntimeException("Exception thrown by mock")) .when(spyClient) @@ -140,9 +142,9 @@ public void testErrorGracefulModeOn() throws JsonProcessingException { @Test public void testErrorGracefulModeOff() { - initClient(TEST_HOST, false, false, DUMMY_FLAG_API_KEY); + initClient(false, false); - BaseEppoClient realClient = BaseEppoClient.getInstance(); + BaseEppoClient realClient = eppoClient; BaseEppoClient spyClient = spy(realClient); doThrow(new RuntimeException("Exception thrown by mock")) .when(spyClient) @@ -198,10 +200,10 @@ public void testInvalidConfigJSON() { mockHttpResponse("{}"); - initClient(TEST_HOST, false, false, DUMMY_FLAG_API_KEY); + initClient(false, false); String result = - BaseEppoClient.getInstance() + eppoClient .getStringAssignment("dummy subject", "dummy flag", "not-populated"); assertEquals("not-populated", result); } @@ -214,7 +216,7 @@ public void testAssignmentEventCorrectlyCreated() { subjectAttributes.put("age", EppoValue.valueOf(30)); subjectAttributes.put("employer", EppoValue.valueOf("Eppo")); double assignment = - BaseEppoClient.getInstance() + eppoClient .getDoubleAssignment("numeric_flag", "alice", subjectAttributes, 0.0); assertEquals(3.1415926, assignment, 0.0000001); @@ -252,7 +254,7 @@ public void testAssignmentLogErrorNonFatal() { .when(mockAssignmentLogger) .logAssignment(any()); double assignment = - BaseEppoClient.getInstance() + eppoClient .getDoubleAssignment("numeric_flag", "alice", new Attributes(), 0.0); assertEquals(3.1415926, assignment, 0.0000001); @@ -261,6 +263,7 @@ public void testAssignmentLogErrorNonFatal() { verify(mockAssignmentLogger, times(1)).logAssignment(assignmentLogCaptor.capture()); } + @SuppressWarnings("SameParameterValue") private void mockHttpResponse(String responseBody) { // Create a mock instance of EppoHttpClient EppoHttpClient mockHttpClient = mock(EppoHttpClient.class); @@ -281,12 +284,12 @@ private void mockHttpResponse(String responseBody) { // Mock async get doAnswer( invocation -> { - RequestCallback callback = invocation.getArgument(1); + EppoHttpClientRequestCallback callback = invocation.getArgument(1); callback.onSuccess(responseBody); return null; // doAnswer doesn't require a return value }) .when(mockHttpClient) - .get(anyString(), any(RequestCallback.class)); + .get(anyString(), any(EppoHttpClientRequestCallback.class)); setHttpClientOverrideField(mockHttpClient); } @@ -295,11 +298,8 @@ private void setHttpClientOverrideField(EppoHttpClient httpClient) { setOverrideField("httpClientOverride", httpClient); } - private void setConfigurationStoreOverrideField(ConfigurationStore configurationStore) { - setOverrideField("configurationStoreOverride", configurationStore); - } - /** Uses reflection to set a static override field used for tests (e.g., httpClientOverride) */ + @SuppressWarnings("SameParameterValue") private void setOverrideField(String fieldName, T override) { try { Field httpClientOverrideField = BaseEppoClient.class.getDeclaredField(fieldName); diff --git a/src/test/java/cloud/eppo/FlagEvaluatorTest.java b/src/test/java/cloud/eppo/FlagEvaluatorTest.java index c8e2dd84..d5effda1 100644 --- a/src/test/java/cloud/eppo/FlagEvaluatorTest.java +++ b/src/test/java/cloud/eppo/FlagEvaluatorTest.java @@ -3,7 +3,6 @@ import static cloud.eppo.Utils.base64Encode; import static cloud.eppo.Utils.getMD5Hex; import static org.junit.jupiter.api.Assertions.*; -import static org.junit.jupiter.api.Assertions.assertEquals; import cloud.eppo.model.ShardRange; import cloud.eppo.ufc.dto.Allocation; diff --git a/src/test/java/cloud/eppo/helpers/AssignmentTestCase.java b/src/test/java/cloud/eppo/helpers/AssignmentTestCase.java index d1857c2d..cea71931 100644 --- a/src/test/java/cloud/eppo/helpers/AssignmentTestCase.java +++ b/src/test/java/cloud/eppo/helpers/AssignmentTestCase.java @@ -16,7 +16,6 @@ import java.util.stream.Stream; import static org.junit.jupiter.api.Assertions.*; -import static org.junit.jupiter.api.Assertions.assertEquals; public class AssignmentTestCase { private final String flag; @@ -60,7 +59,7 @@ public String getFileName() { return fileName; } - private static ObjectMapper mapper = new ObjectMapper().registerModule(assignmentTestCaseModule()); + private static final ObjectMapper mapper = new ObjectMapper().registerModule(assignmentTestCaseModule()); public static SimpleModule assignmentTestCaseModule() { SimpleModule module = new SimpleModule(); @@ -92,10 +91,9 @@ public static AssignmentTestCase parseTestCaseFile(File testCaseFile) { return testCase; } - public static void runTestCase(AssignmentTestCase testCase) { + public static void runTestCase(AssignmentTestCase testCase, BaseEppoClient eppoClient) { String flagKey = testCase.getFlag(); TestCaseValue defaultValue = testCase.getDefaultValue(); - BaseEppoClient eppoClient = BaseEppoClient.getInstance(); assertFalse(testCase.getSubjects().isEmpty()); for (SubjectAssignment subjectAssignment : testCase.getSubjects()) { From b964d839defe3050471cc89eca99e1560df0982d Mon Sep 17 00:00:00 2001 From: Aaron Silverman Date: Tue, 13 Aug 2024 10:18:12 -0600 Subject: [PATCH 3/7] linter --- src/main/java/cloud/eppo/BaseEppoClient.java | 5 +- src/main/java/cloud/eppo/EppoHttpClient.java | 8 +- .../eppo/EppoHttpClientRequestCallback.java | 1 + .../cloud/eppo/BaseEppoClientBanditTest.java | 51 +++++----- .../java/cloud/eppo/BaseEppoClientTest.java | 34 +++---- .../eppo/helpers/AssignmentTestCase.java | 92 +++++++++---------- 6 files changed, 91 insertions(+), 100 deletions(-) diff --git a/src/main/java/cloud/eppo/BaseEppoClient.java b/src/main/java/cloud/eppo/BaseEppoClient.java index fc9ef7ed..9079240f 100644 --- a/src/main/java/cloud/eppo/BaseEppoClient.java +++ b/src/main/java/cloud/eppo/BaseEppoClient.java @@ -48,15 +48,14 @@ public BaseEppoClient( AssignmentLogger assignmentLogger, BanditLogger banditLogger, boolean isGracefulMode, - boolean expectObfuscatedConfig - ) { + boolean expectObfuscatedConfig) { if (apiKey == null) { throw new IllegalArgumentException("Unable to initialize Eppo SDK due to missing API key"); } if (sdkName == null || sdkVersion == null) { throw new IllegalArgumentException( - "Unable to initialize Eppo SDK due to missing SDK name or version"); + "Unable to initialize Eppo SDK due to missing SDK name or version"); } if (host == null) { host = DEFAULT_HOST; diff --git a/src/main/java/cloud/eppo/EppoHttpClient.java b/src/main/java/cloud/eppo/EppoHttpClient.java index 8cc81daa..54d66db4 100644 --- a/src/main/java/cloud/eppo/EppoHttpClient.java +++ b/src/main/java/cloud/eppo/EppoHttpClient.java @@ -95,11 +95,13 @@ public void onResponse(Call call, Response response) { @Override public void onFailure(Call call, IOException e) { - log.error("Http request failure: {} {}", e.getMessage(), Arrays.toString(e.getStackTrace()), e); + log.error( + "Http request failure: {} {}", + e.getMessage(), + Arrays.toString(e.getStackTrace()), + e); callback.onFailure("Unable to fetch from URL " + httpUrl); } }); } } - - diff --git a/src/main/java/cloud/eppo/EppoHttpClientRequestCallback.java b/src/main/java/cloud/eppo/EppoHttpClientRequestCallback.java index 9258219b..6d67a2c4 100644 --- a/src/main/java/cloud/eppo/EppoHttpClientRequestCallback.java +++ b/src/main/java/cloud/eppo/EppoHttpClientRequestCallback.java @@ -2,5 +2,6 @@ public interface EppoHttpClientRequestCallback { void onSuccess(String responseBody); + void onFailure(String errorMessage); } diff --git a/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java b/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java index 86cd45d4..7f7d8322 100644 --- a/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java +++ b/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java @@ -45,16 +45,16 @@ public class BaseEppoClientBanditTest { @BeforeAll public static void initClient() { - eppoClient = new BaseEppoClient( - DUMMY_BANDIT_API_KEY, - "java", - "3.0.0", - TEST_HOST, - mockAssignmentLogger, - mockBanditLogger, - false, - false - ); + eppoClient = + new BaseEppoClient( + DUMMY_BANDIT_API_KEY, + "java", + "3.0.0", + TEST_HOST, + mockAssignmentLogger, + mockBanditLogger, + false, + false); eppoClient.loadConfiguration(); @@ -110,8 +110,7 @@ private void runBanditTestCase(BanditTestCase testCase) { ContextAttributes attributes = subjectAssignment.getSubjectAttributes(); Actions actions = subjectAssignment.getActions(); BanditResult assignment = - eppoClient - .getBanditAction(flagKey, subjectKey, attributes, actions, defaultValue); + eppoClient.getBanditAction(flagKey, subjectKey, attributes, actions, defaultValue); assertBanditAssignment(flagKey, subjectAssignment, assignment); } } @@ -170,8 +169,7 @@ public void testBanditLogsAction() { actions.put("reebok", rebookAttributes); BanditResult banditResult = - eppoClient - .getBanditAction(flagKey, subjectKey, subjectAttributes, actions, "control"); + eppoClient.getBanditAction(flagKey, subjectKey, subjectAttributes, actions, "control"); // Verify assignment assertEquals("banner_bandit", banditResult.getVariation()); @@ -244,8 +242,7 @@ public void testNoBanditLogsWhenNotBandit() { actions.put("adidas", new Attributes()); BanditResult banditResult = - eppoClient - .getBanditAction(flagKey, subjectKey, subjectAttributes, actions, "default"); + eppoClient.getBanditAction(flagKey, subjectKey, subjectAttributes, actions, "default"); // Verify assignment assertEquals("control", banditResult.getVariation()); @@ -272,8 +269,7 @@ public void testNoBanditLogsWhenNoActions() { BanditActions actions = new BanditActions(); BanditResult banditResult = - eppoClient - .getBanditAction(flagKey, subjectKey, subjectAttributes, actions, "control"); + eppoClient.getBanditAction(flagKey, subjectKey, subjectAttributes, actions, "control"); // Verify assignment assertEquals("banner_bandit", banditResult.getVariation()); @@ -291,9 +287,8 @@ public void testNoBanditLogsWhenNoActions() { @Test public void testBanditErrorGracefulModeOff() { - eppoClient - .setIsGracefulFailureMode( - false); // Should be set by @BeforeEach but repeated here for test clarity + eppoClient.setIsGracefulFailureMode( + false); // Should be set by @BeforeEach but repeated here for test clarity try (MockedStatic mockedStatic = mockStatic(BanditEvaluator.class)) { // Configure the mock to throw an exception mockedStatic @@ -307,9 +302,8 @@ public void testBanditErrorGracefulModeOff() { assertThrows( RuntimeException.class, () -> - eppoClient - .getBanditAction( - "banner_bandit_flag", "subject", new Attributes(), actions, "default")); + eppoClient.getBanditAction( + "banner_bandit_flag", "subject", new Attributes(), actions, "default")); } } @@ -327,9 +321,8 @@ public void testBanditErrorGracefulModeOn() { actions.put("nike", new Attributes()); actions.put("adidas", new Attributes()); BanditResult banditResult = - eppoClient - .getBanditAction( - "banner_bandit_flag", "subject", new Attributes(), actions, "default"); + eppoClient.getBanditAction( + "banner_bandit_flag", "subject", new Attributes(), actions, "default"); assertEquals("banner_bandit", banditResult.getVariation()); assertNull(banditResult.getAction()); } @@ -346,8 +339,8 @@ public void testBanditLogErrorNonFatal() { actions.put("nike", new Attributes()); actions.put("adidas", new Attributes()); BanditResult banditResult = - eppoClient - .getBanditAction("banner_bandit_flag", "subject", new Attributes(), actions, "default"); + eppoClient.getBanditAction( + "banner_bandit_flag", "subject", new Attributes(), actions, "default"); assertEquals("banner_bandit", banditResult.getVariation()); assertEquals("nike", banditResult.getAction()); diff --git a/src/test/java/cloud/eppo/BaseEppoClientTest.java b/src/test/java/cloud/eppo/BaseEppoClientTest.java index cf290273..609ae04c 100644 --- a/src/test/java/cloud/eppo/BaseEppoClientTest.java +++ b/src/test/java/cloud/eppo/BaseEppoClientTest.java @@ -6,7 +6,6 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; - import static org.mockito.Mockito.*; import cloud.eppo.helpers.AssignmentTestCase; @@ -36,7 +35,8 @@ public class BaseEppoClientTest { private static final String DUMMY_FLAG_API_KEY = "dummy-flags-api-key"; // Will load flags-v1 private static final String TEST_HOST = "https://us-central1-eppo-qa.cloudfunctions.net/serveGitHubRacTestFile"; - private final ObjectMapper mapper = new ObjectMapper().registerModule(AssignmentTestCase.assignmentTestCaseModule()); + private final ObjectMapper mapper = + new ObjectMapper().registerModule(AssignmentTestCase.assignmentTestCaseModule()); private BaseEppoClient eppoClient; private AssignmentLogger mockAssignmentLogger; @@ -50,16 +50,16 @@ private void initClient() { private void initClient(boolean isGracefulMode, boolean isConfigObfuscated) { mockAssignmentLogger = mock(AssignmentLogger.class); - eppoClient = new BaseEppoClient( - DUMMY_FLAG_API_KEY, - isConfigObfuscated ? "android" : "java", - "3.0.0", - TEST_HOST, - mockAssignmentLogger, - null, - isGracefulMode, - isConfigObfuscated - ); + eppoClient = + new BaseEppoClient( + DUMMY_FLAG_API_KEY, + isConfigObfuscated ? "android" : "java", + "3.0.0", + TEST_HOST, + mockAssignmentLogger, + null, + isGracefulMode, + isConfigObfuscated); eppoClient.loadConfiguration(); log.info("Test client initialized"); @@ -202,9 +202,7 @@ public void testInvalidConfigJSON() { initClient(false, false); - String result = - eppoClient - .getStringAssignment("dummy subject", "dummy flag", "not-populated"); + String result = eppoClient.getStringAssignment("dummy subject", "dummy flag", "not-populated"); assertEquals("not-populated", result); } @@ -216,8 +214,7 @@ public void testAssignmentEventCorrectlyCreated() { subjectAttributes.put("age", EppoValue.valueOf(30)); subjectAttributes.put("employer", EppoValue.valueOf("Eppo")); double assignment = - eppoClient - .getDoubleAssignment("numeric_flag", "alice", subjectAttributes, 0.0); + eppoClient.getDoubleAssignment("numeric_flag", "alice", subjectAttributes, 0.0); assertEquals(3.1415926, assignment, 0.0000001); @@ -254,8 +251,7 @@ public void testAssignmentLogErrorNonFatal() { .when(mockAssignmentLogger) .logAssignment(any()); double assignment = - eppoClient - .getDoubleAssignment("numeric_flag", "alice", new Attributes(), 0.0); + eppoClient.getDoubleAssignment("numeric_flag", "alice", new Attributes(), 0.0); assertEquals(3.1415926, assignment, 0.0000001); diff --git a/src/test/java/cloud/eppo/helpers/AssignmentTestCase.java b/src/test/java/cloud/eppo/helpers/AssignmentTestCase.java index cea71931..7206621e 100644 --- a/src/test/java/cloud/eppo/helpers/AssignmentTestCase.java +++ b/src/test/java/cloud/eppo/helpers/AssignmentTestCase.java @@ -1,21 +1,20 @@ package cloud.eppo.helpers; +import static org.junit.jupiter.api.Assertions.*; + import cloud.eppo.BaseEppoClient; import cloud.eppo.ufc.dto.Attributes; import cloud.eppo.ufc.dto.VariationType; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.module.SimpleModule; -import org.apache.commons.io.FileUtils; -import org.junit.jupiter.params.provider.Arguments; - import java.io.File; import java.io.IOException; import java.util.ArrayList; import java.util.List; import java.util.stream.Stream; - -import static org.junit.jupiter.api.Assertions.*; +import org.apache.commons.io.FileUtils; +import org.junit.jupiter.params.provider.Arguments; public class AssignmentTestCase { private final String flag; @@ -59,7 +58,8 @@ public String getFileName() { return fileName; } - private static final ObjectMapper mapper = new ObjectMapper().registerModule(assignmentTestCaseModule()); + private static final ObjectMapper mapper = + new ObjectMapper().registerModule(assignmentTestCaseModule()); public static SimpleModule assignmentTestCaseModule() { SimpleModule module = new SimpleModule(); @@ -105,91 +105,91 @@ public static void runTestCase(AssignmentTestCase testCase, BaseEppoClient eppoC switch (testCase.getVariationType()) { case BOOLEAN: boolean boolAssignment = - eppoClient.getBooleanAssignment( - flagKey, subjectKey, subjectAttributes, defaultValue.booleanValue()); + eppoClient.getBooleanAssignment( + flagKey, subjectKey, subjectAttributes, defaultValue.booleanValue()); assertAssignment(flagKey, subjectAssignment, boolAssignment); break; case INTEGER: int intAssignment = - eppoClient.getIntegerAssignment( - flagKey, - subjectKey, - subjectAttributes, - Double.valueOf(defaultValue.doubleValue()).intValue()); + eppoClient.getIntegerAssignment( + flagKey, + subjectKey, + subjectAttributes, + Double.valueOf(defaultValue.doubleValue()).intValue()); assertAssignment(flagKey, subjectAssignment, intAssignment); break; case NUMERIC: double doubleAssignment = - eppoClient.getDoubleAssignment( - flagKey, subjectKey, subjectAttributes, defaultValue.doubleValue()); + eppoClient.getDoubleAssignment( + flagKey, subjectKey, subjectAttributes, defaultValue.doubleValue()); assertAssignment(flagKey, subjectAssignment, doubleAssignment); break; case STRING: String stringAssignment = - eppoClient.getStringAssignment( - flagKey, subjectKey, subjectAttributes, defaultValue.stringValue()); + eppoClient.getStringAssignment( + flagKey, subjectKey, subjectAttributes, defaultValue.stringValue()); assertAssignment(flagKey, subjectAssignment, stringAssignment); break; case JSON: JsonNode jsonAssignment = - eppoClient.getJSONAssignment( - flagKey, subjectKey, subjectAttributes, testCase.getDefaultValue().jsonValue()); + eppoClient.getJSONAssignment( + flagKey, subjectKey, subjectAttributes, testCase.getDefaultValue().jsonValue()); assertAssignment(flagKey, subjectAssignment, jsonAssignment); break; default: throw new UnsupportedOperationException( - "Unexpected variation type " - + testCase.getVariationType() - + " for " - + flagKey - + " test case"); + "Unexpected variation type " + + testCase.getVariationType() + + " for " + + flagKey + + " test case"); } } } /** Helper method for asserting a subject assignment with a useful failure message. */ public static void assertAssignment( - String flagKey, SubjectAssignment expectedSubjectAssignment, T assignment) { + String flagKey, SubjectAssignment expectedSubjectAssignment, T assignment) { if (assignment == null) { fail( - "Unexpected null " - + flagKey - + " assignment for subject " - + expectedSubjectAssignment.getSubjectKey()); + "Unexpected null " + + flagKey + + " assignment for subject " + + expectedSubjectAssignment.getSubjectKey()); } String failureMessage = - "Incorrect " - + flagKey - + " assignment for subject " - + expectedSubjectAssignment.getSubjectKey(); + "Incorrect " + + flagKey + + " assignment for subject " + + expectedSubjectAssignment.getSubjectKey(); if (assignment instanceof Boolean) { assertEquals( - expectedSubjectAssignment.getAssignment().booleanValue(), assignment, failureMessage); + expectedSubjectAssignment.getAssignment().booleanValue(), assignment, failureMessage); } else if (assignment instanceof Integer) { assertEquals( - Double.valueOf(expectedSubjectAssignment.getAssignment().doubleValue()).intValue(), - assignment, - failureMessage); + Double.valueOf(expectedSubjectAssignment.getAssignment().doubleValue()).intValue(), + assignment, + failureMessage); } else if (assignment instanceof Double) { assertEquals( - expectedSubjectAssignment.getAssignment().doubleValue(), - (Double) assignment, - 0.000001, - failureMessage); + expectedSubjectAssignment.getAssignment().doubleValue(), + (Double) assignment, + 0.000001, + failureMessage); } else if (assignment instanceof String) { assertEquals( - expectedSubjectAssignment.getAssignment().stringValue(), assignment, failureMessage); + expectedSubjectAssignment.getAssignment().stringValue(), assignment, failureMessage); } else if (assignment instanceof JsonNode) { assertEquals( - expectedSubjectAssignment.getAssignment().jsonValue().toString(), - assignment.toString(), - failureMessage); + expectedSubjectAssignment.getAssignment().jsonValue().toString(), + assignment.toString(), + failureMessage); } else { throw new IllegalArgumentException( - "Unexpected assignment type " + assignment.getClass().getCanonicalName()); + "Unexpected assignment type " + assignment.getClass().getCanonicalName()); } } } From 58e28d6767d4f7743eb390b369c682bcb4a539fa Mon Sep 17 00:00:00 2001 From: Aaron Silverman Date: Tue, 13 Aug 2024 10:58:50 -0600 Subject: [PATCH 4/7] expose bandit test harnesses --- .../cloud/eppo/BaseEppoClientBanditTest.java | 82 ++--------------- .../eppo/helpers/AssignmentTestCase.java | 11 +-- .../cloud/eppo/helpers/BanditTestCase.java | 89 ++++++++++++++++++- 3 files changed, 92 insertions(+), 90 deletions(-) diff --git a/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java b/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java index 7f7d8322..5d93c1af 100644 --- a/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java +++ b/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java @@ -1,5 +1,7 @@ package cloud.eppo; +import static cloud.eppo.helpers.BanditTestCase.parseBanditTestCaseFile; +import static cloud.eppo.helpers.BanditTestCase.runBanditTestCase; import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.*; @@ -9,13 +11,9 @@ import cloud.eppo.logging.BanditAssignment; import cloud.eppo.logging.BanditLogger; import cloud.eppo.ufc.dto.*; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.module.SimpleModule; import java.io.File; -import java.io.IOException; import java.util.*; import java.util.stream.Stream; -import org.apache.commons.io.FileUtils; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -33,7 +31,6 @@ public class BaseEppoClientBanditTest { "dummy-bandits-api-key"; // Will load bandit-flags-v1 private static final String TEST_HOST = "https://us-central1-eppo-qa.cloudfunctions.net/serveGitHubRacTestFile"; - private static final ObjectMapper mapper = new ObjectMapper().registerModule(module()); private static final AssignmentLogger mockAssignmentLogger = mock(AssignmentLogger.class); private static final BanditLogger mockBanditLogger = mock(BanditLogger.class); @@ -72,73 +69,12 @@ public void reset() { @ParameterizedTest @MethodSource("getBanditTestData") public void testUnobfuscatedBanditAssignments(File testFile) { - BanditTestCase testCase = parseTestCaseFile(testFile); - runBanditTestCase(testCase); + BanditTestCase testCase = parseBanditTestCaseFile(testFile); + runBanditTestCase(testCase, eppoClient); } - private static Stream getBanditTestData() { - File testCaseFolder = new File("src/test/resources/shared/ufc/bandit-tests"); - File[] testCaseFiles = testCaseFolder.listFiles(); - assertNotNull(testCaseFiles); - assertTrue(testCaseFiles.length > 0); - List arguments = new ArrayList<>(); - for (File testCaseFile : testCaseFiles) { - arguments.add(Arguments.of(testCaseFile)); - } - return arguments.stream(); - } - - private BanditTestCase parseTestCaseFile(File testCaseFile) { - BanditTestCase testCase; - try { - String json = FileUtils.readFileToString(testCaseFile, "UTF8"); - testCase = mapper.readValue(json, BanditTestCase.class); - } catch (IOException ex) { - throw new RuntimeException(ex); - } - return testCase; - } - - private void runBanditTestCase(BanditTestCase testCase) { - assertFalse(testCase.getSubjects().isEmpty()); - - String flagKey = testCase.getFlag(); - String defaultValue = testCase.getDefaultValue(); - - for (BanditSubjectAssignment subjectAssignment : testCase.getSubjects()) { - String subjectKey = subjectAssignment.getSubjectKey(); - ContextAttributes attributes = subjectAssignment.getSubjectAttributes(); - Actions actions = subjectAssignment.getActions(); - BanditResult assignment = - eppoClient.getBanditAction(flagKey, subjectKey, attributes, actions, defaultValue); - assertBanditAssignment(flagKey, subjectAssignment, assignment); - } - } - - /** Helper method for asserting a bandit assignment with a useful failure message. */ - private void assertBanditAssignment( - String flagKey, BanditSubjectAssignment expectedSubjectAssignment, BanditResult assignment) { - String failureMessage = - "Incorrect " - + flagKey - + " variation assignment for subject " - + expectedSubjectAssignment.getSubjectKey(); - - assertEquals( - expectedSubjectAssignment.getAssignment().getVariation(), - assignment.getVariation(), - failureMessage); - - failureMessage = - "Incorrect " - + flagKey - + " action assignment for subject " - + expectedSubjectAssignment.getSubjectKey(); - - assertEquals( - expectedSubjectAssignment.getAssignment().getAction(), - assignment.getAction(), - failureMessage); + public static Stream getBanditTestData() { + return BanditTestCase.getBanditTestData(); } @SuppressWarnings("ExtractMethodRecommender") @@ -348,10 +284,4 @@ public void testBanditLogErrorNonFatal() { ArgumentCaptor.forClass(BanditAssignment.class); verify(mockBanditLogger, times(1)).logBanditAssignment(banditLogCaptor.capture()); } - - private static SimpleModule module() { - SimpleModule module = new SimpleModule(); - module.addDeserializer(BanditTestCase.class, new BanditTestCaseDeserializer()); - return module; - } } diff --git a/src/test/java/cloud/eppo/helpers/AssignmentTestCase.java b/src/test/java/cloud/eppo/helpers/AssignmentTestCase.java index 7206621e..4513f359 100644 --- a/src/test/java/cloud/eppo/helpers/AssignmentTestCase.java +++ b/src/test/java/cloud/eppo/helpers/AssignmentTestCase.java @@ -21,7 +21,6 @@ public class AssignmentTestCase { private final VariationType variationType; private final TestCaseValue defaultValue; private final List subjects; - private String fileName; public AssignmentTestCase( String flag, @@ -50,14 +49,6 @@ public List getSubjects() { return subjects; } - public void setFileName(String fileName) { - this.fileName = fileName; - } - - public String getFileName() { - return fileName; - } - private static final ObjectMapper mapper = new ObjectMapper().registerModule(assignmentTestCaseModule()); @@ -148,7 +139,7 @@ public static void runTestCase(AssignmentTestCase testCase, BaseEppoClient eppoC } /** Helper method for asserting a subject assignment with a useful failure message. */ - public static void assertAssignment( + private static void assertAssignment( String flagKey, SubjectAssignment expectedSubjectAssignment, T assignment) { if (assignment == null) { diff --git a/src/test/java/cloud/eppo/helpers/BanditTestCase.java b/src/test/java/cloud/eppo/helpers/BanditTestCase.java index 90e6ff86..5c6efe67 100644 --- a/src/test/java/cloud/eppo/helpers/BanditTestCase.java +++ b/src/test/java/cloud/eppo/helpers/BanditTestCase.java @@ -1,6 +1,21 @@ package cloud.eppo.helpers; +import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import cloud.eppo.BaseEppoClient; +import cloud.eppo.ufc.dto.Actions; +import cloud.eppo.ufc.dto.BanditResult; +import cloud.eppo.ufc.dto.ContextAttributes; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.module.SimpleModule; +import java.io.File; +import java.io.IOException; +import java.util.ArrayList; import java.util.List; +import java.util.stream.Stream; +import org.apache.commons.io.FileUtils; +import org.junit.jupiter.params.provider.Arguments; public class BanditTestCase { private final String flag; @@ -26,11 +41,77 @@ public List getSubjects() { return subjects; } - public void setFileName(String fileName) { - this.fileName = fileName; + public static Stream getBanditTestData() { + File testCaseFolder = new File("src/test/resources/shared/ufc/bandit-tests"); + File[] testCaseFiles = testCaseFolder.listFiles(); + assertNotNull(testCaseFiles); + assertTrue(testCaseFiles.length > 0); + List arguments = new ArrayList<>(); + for (File testCaseFile : testCaseFiles) { + arguments.add(Arguments.of(testCaseFile)); + } + return arguments.stream(); + } + + private static final ObjectMapper mapper = + new ObjectMapper().registerModule(banditTestCaseModule()); + + public static SimpleModule banditTestCaseModule() { + SimpleModule module = new SimpleModule(); + module.addDeserializer(BanditTestCase.class, new BanditTestCaseDeserializer()); + return module; } - public String getFileName() { - return fileName; + public static BanditTestCase parseBanditTestCaseFile(File testCaseFile) { + BanditTestCase testCase; + try { + String json = FileUtils.readFileToString(testCaseFile, "UTF8"); + testCase = mapper.readValue(json, BanditTestCase.class); + } catch (IOException ex) { + throw new RuntimeException(ex); + } + return testCase; + } + + public static void runBanditTestCase(BanditTestCase testCase, BaseEppoClient eppoClient) { + assertFalse(testCase.getSubjects().isEmpty()); + + String flagKey = testCase.getFlag(); + String defaultValue = testCase.getDefaultValue(); + + for (BanditSubjectAssignment subjectAssignment : testCase.getSubjects()) { + String subjectKey = subjectAssignment.getSubjectKey(); + ContextAttributes attributes = subjectAssignment.getSubjectAttributes(); + Actions actions = subjectAssignment.getActions(); + BanditResult assignment = + eppoClient.getBanditAction(flagKey, subjectKey, attributes, actions, defaultValue); + assertBanditAssignment(flagKey, subjectAssignment, assignment); + } + } + + /** Helper method for asserting a bandit assignment with a useful failure message. */ + private static void assertBanditAssignment( + String flagKey, BanditSubjectAssignment expectedSubjectAssignment, BanditResult assignment) { + String failureMessage = + "Incorrect " + + flagKey + + " variation assignment for subject " + + expectedSubjectAssignment.getSubjectKey(); + + assertEquals( + expectedSubjectAssignment.getAssignment().getVariation(), + assignment.getVariation(), + failureMessage); + + failureMessage = + "Incorrect " + + flagKey + + " action assignment for subject " + + expectedSubjectAssignment.getSubjectKey(); + + assertEquals( + expectedSubjectAssignment.getAssignment().getAction(), + assignment.getAction(), + failureMessage); } } From 5e856bedb2d8392aa64034a9f20842d645b17814 Mon Sep 17 00:00:00 2001 From: Aaron Silverman Date: Tue, 13 Aug 2024 15:00:07 -0600 Subject: [PATCH 5/7] expose test uilities --- src/main/java/cloud/eppo/BaseEppoClient.java | 1 - .../java/cloud/eppo/BaseEppoClientTest.java | 56 ++--------------- .../{UtilsTest.java => TestUtilsTest.java} | 2 +- .../java/cloud/eppo/helpers/TestUtils.java | 62 +++++++++++++++++++ 4 files changed, 67 insertions(+), 54 deletions(-) rename src/test/java/cloud/eppo/{UtilsTest.java => TestUtilsTest.java} (96%) create mode 100644 src/test/java/cloud/eppo/helpers/TestUtils.java diff --git a/src/main/java/cloud/eppo/BaseEppoClient.java b/src/main/java/cloud/eppo/BaseEppoClient.java index 9079240f..363c0895 100644 --- a/src/main/java/cloud/eppo/BaseEppoClient.java +++ b/src/main/java/cloud/eppo/BaseEppoClient.java @@ -24,7 +24,6 @@ public class BaseEppoClient { .registerModule(EppoModule.eppoModule()); // TODO: is this the best place for this? protected static final String DEFAULT_HOST = "https://fscdn.eppo.cloud"; - protected static final boolean DEFAULT_IS_GRACEFUL_MODE = true; protected final ConfigurationRequestor requestor; private final ConfigurationStore configurationStore; diff --git a/src/test/java/cloud/eppo/BaseEppoClientTest.java b/src/test/java/cloud/eppo/BaseEppoClientTest.java index 609ae04c..80670daf 100644 --- a/src/test/java/cloud/eppo/BaseEppoClientTest.java +++ b/src/test/java/cloud/eppo/BaseEppoClientTest.java @@ -2,6 +2,8 @@ import static cloud.eppo.helpers.AssignmentTestCase.parseTestCaseFile; import static cloud.eppo.helpers.AssignmentTestCase.runTestCase; +import static cloud.eppo.helpers.TestUtils.mockHttpResponse; +import static cloud.eppo.helpers.TestUtils.setBaseClientHttpClientOverrideField; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -17,10 +19,8 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import java.io.File; -import java.lang.reflect.Field; import java.util.*; import java.util.stream.Stream; -import okhttp3.*; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -68,7 +68,7 @@ private void initClient(boolean isGracefulMode, boolean isConfigObfuscated) { @BeforeEach public void cleanUp() { // TODO: Clear any caches - setHttpClientOverrideField(null); + setBaseClientHttpClientOverrideField(null); } @ParameterizedTest @@ -198,7 +198,7 @@ public void testErrorGracefulModeOff() { @Test public void testInvalidConfigJSON() { - mockHttpResponse("{}"); + mockHttpResponse(TEST_HOST, "{}"); initClient(false, false); @@ -259,53 +259,5 @@ public void testAssignmentLogErrorNonFatal() { verify(mockAssignmentLogger, times(1)).logAssignment(assignmentLogCaptor.capture()); } - @SuppressWarnings("SameParameterValue") - private void mockHttpResponse(String responseBody) { - // Create a mock instance of EppoHttpClient - EppoHttpClient mockHttpClient = mock(EppoHttpClient.class); - - // Mock sync get - Response dummyResponse = - new Response.Builder() - // Used by test - .code(200) - .body(ResponseBody.create(responseBody, MediaType.get("application/json"))) - // Below properties are required to build the Response (but unused) - .request(new Request.Builder().url(TEST_HOST).build()) - .protocol(Protocol.HTTP_1_1) - .message("OK") - .build(); - when(mockHttpClient.get(anyString())).thenReturn(dummyResponse); - - // Mock async get - doAnswer( - invocation -> { - EppoHttpClientRequestCallback callback = invocation.getArgument(1); - callback.onSuccess(responseBody); - return null; // doAnswer doesn't require a return value - }) - .when(mockHttpClient) - .get(anyString(), any(EppoHttpClientRequestCallback.class)); - - setHttpClientOverrideField(mockHttpClient); - } - - private void setHttpClientOverrideField(EppoHttpClient httpClient) { - setOverrideField("httpClientOverride", httpClient); - } - - /** Uses reflection to set a static override field used for tests (e.g., httpClientOverride) */ - @SuppressWarnings("SameParameterValue") - private void setOverrideField(String fieldName, T override) { - try { - Field httpClientOverrideField = BaseEppoClient.class.getDeclaredField(fieldName); - httpClientOverrideField.setAccessible(true); - httpClientOverrideField.set(null, override); - httpClientOverrideField.setAccessible(false); - } catch (NoSuchFieldException | IllegalAccessException e) { - throw new RuntimeException(e); - } - } - // TODO: tests for the cache } diff --git a/src/test/java/cloud/eppo/UtilsTest.java b/src/test/java/cloud/eppo/TestUtilsTest.java similarity index 96% rename from src/test/java/cloud/eppo/UtilsTest.java rename to src/test/java/cloud/eppo/TestUtilsTest.java index 892b11e6..a0372a88 100644 --- a/src/test/java/cloud/eppo/UtilsTest.java +++ b/src/test/java/cloud/eppo/TestUtilsTest.java @@ -10,7 +10,7 @@ import java.util.Date; import org.junit.jupiter.api.Test; -public class UtilsTest { +public class TestUtilsTest { @Test public void testParseUtcISODateNode() throws JsonProcessingException { ObjectMapper mapper = new ObjectMapper(); diff --git a/src/test/java/cloud/eppo/helpers/TestUtils.java b/src/test/java/cloud/eppo/helpers/TestUtils.java new file mode 100644 index 00000000..ab0c65e6 --- /dev/null +++ b/src/test/java/cloud/eppo/helpers/TestUtils.java @@ -0,0 +1,62 @@ +package cloud.eppo.helpers; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.*; + +import cloud.eppo.BaseEppoClient; +import cloud.eppo.EppoHttpClient; +import cloud.eppo.EppoHttpClientRequestCallback; +import java.lang.reflect.Field; +import okhttp3.*; + +public class TestUtils { + + @SuppressWarnings("SameParameterValue") + public static void mockHttpResponse(String host, String responseBody) { + // Create a mock instance of EppoHttpClient + EppoHttpClient mockHttpClient = mock(EppoHttpClient.class); + + // Mock sync get + Response dummyResponse = + new Response.Builder() + // Used by test + .code(200) + .body(ResponseBody.create(responseBody, MediaType.get("application/json"))) + // Below properties are required to build the Response (but unused) + .request(new Request.Builder().url(host).build()) + .protocol(Protocol.HTTP_1_1) + .message("OK") + .build(); + when(mockHttpClient.get(anyString())).thenReturn(dummyResponse); + + // Mock async get + doAnswer( + invocation -> { + EppoHttpClientRequestCallback callback = invocation.getArgument(1); + callback.onSuccess(responseBody); + return null; // doAnswer doesn't require a return value + }) + .when(mockHttpClient) + .get(anyString(), any(EppoHttpClientRequestCallback.class)); + + setBaseClientHttpClientOverrideField(mockHttpClient); + } + + public static void setBaseClientHttpClientOverrideField(EppoHttpClient httpClient) { + setBaseClientOverrideField("httpClientOverride", httpClient); + } + + /** Uses reflection to set a static override field used for tests (e.g., httpClientOverride) */ + @SuppressWarnings("SameParameterValue") + public static void setBaseClientOverrideField(String fieldName, T override) { + try { + Field httpClientOverrideField = BaseEppoClient.class.getDeclaredField(fieldName); + httpClientOverrideField.setAccessible(true); + httpClientOverrideField.set(null, override); + httpClientOverrideField.setAccessible(false); + } catch (NoSuchFieldException | IllegalAccessException e) { + throw new RuntimeException(e); + } + } +} From b10ffb009c10c68dd00dd1d0292f5a9c72369822 Mon Sep 17 00:00:00 2001 From: Aaron Silverman Date: Tue, 13 Aug 2024 15:47:54 -0600 Subject: [PATCH 6/7] changes from self-review of PR --- src/test/java/cloud/eppo/{TestUtilsTest.java => UtilsTest.java} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename src/test/java/cloud/eppo/{TestUtilsTest.java => UtilsTest.java} (96%) diff --git a/src/test/java/cloud/eppo/TestUtilsTest.java b/src/test/java/cloud/eppo/UtilsTest.java similarity index 96% rename from src/test/java/cloud/eppo/TestUtilsTest.java rename to src/test/java/cloud/eppo/UtilsTest.java index a0372a88..892b11e6 100644 --- a/src/test/java/cloud/eppo/TestUtilsTest.java +++ b/src/test/java/cloud/eppo/UtilsTest.java @@ -10,7 +10,7 @@ import java.util.Date; import org.junit.jupiter.api.Test; -public class TestUtilsTest { +public class UtilsTest { @Test public void testParseUtcISODateNode() throws JsonProcessingException { ObjectMapper mapper = new ObjectMapper(); From 1519ba61d23b6cddf1554703b8251adc89a5bd23 Mon Sep 17 00:00:00 2001 From: Aaron Silverman Date: Wed, 14 Aug 2024 17:03:04 -0600 Subject: [PATCH 7/7] make base client constructor protected --- src/main/java/cloud/eppo/BaseEppoClient.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/cloud/eppo/BaseEppoClient.java b/src/main/java/cloud/eppo/BaseEppoClient.java index 363c0895..8ed889ec 100644 --- a/src/main/java/cloud/eppo/BaseEppoClient.java +++ b/src/main/java/cloud/eppo/BaseEppoClient.java @@ -39,7 +39,7 @@ public class BaseEppoClient { /** @noinspection FieldMayBeFinal */ private static EppoHttpClient httpClientOverride = null; - public BaseEppoClient( + protected BaseEppoClient( String apiKey, String sdkName, String sdkVersion,