feat: add pluggable Base64Codec interface for Android compatibility#223
Conversation
- EppoConfigurationRequest: @NotNull on getters - EppoConfigurationRequestFactory: @NotNull on factory method returns - EppoConfigurationResponse: @Nullable/@NotNull on constructor, factory methods, and getters - ConfigurationParser: @NotNull on method params and returns - EppoConfigurationClient: @NotNull on get() method
… the ConfigurationRequestor
Add pluggable Base64 codec support to allow platform-specific implementations (e.g., Android SDK using android.util.Base64). - Add public Base64Codec interface with encode/decode methods - Add setBase64Codec() method for custom codec injection - Create DefaultBase64Codec inner class using java.util.Base64 - Delegate existing base64Encode/base64Decode methods to codec
Remove duplicate base64Decode implementation from FlagConfigResponseDeserializer and use the centralized version from Utils via static import.
Add tests to verify Base64 codec pluggability: - testCustomBase64Codec: verifies custom codec injection works - testBase64EncodeDecodeDefault: verifies default codec behavior - @AfterEach reset method ensures test isolation
- Add volatile keyword to base64Codec field for thread safety - Add null check in setBase64Codec() throwing IllegalArgumentException - Add resetBase64Codec() method for test cleanup - Use explicit StandardCharsets.UTF_8 in base64Decode - Add test for null codec rejection - Simplify test reset to use resetBase64Codec method
There was a problem hiding this comment.
Pull request overview
This PR introduces a pluggable Base64 codec interface to enable Android compatibility. The java.util.Base64 class is only available on Android API 26+, so this change allows platform-specific implementations (e.g., using android.util.Base64 for older Android versions) to be injected at runtime.
Changes:
- Added
Base64Codecpublic interface with encoding/decoding methods and a setter for custom implementations - Refactored existing Base64 logic into a
DefaultBase64Codecimplementation that maintains backward compatibility - Removed duplicate
base64Decodeimplementation fromFlagConfigResponseDeserializerto use centralized Utils method - Added comprehensive test coverage for custom codec injection, default behavior, and null handling
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/main/java/cloud/eppo/Utils.java | Added Base64Codec interface, setBase64Codec() setter, resetBase64Codec() for testing, and DefaultBase64Codec implementation with improved charset handling |
| eppo-sdk-common/src/main/java/cloud/eppo/ufc/dto/adapters/FlagConfigResponseDeserializer.java | Removed duplicate base64Decode implementation in favor of centralized Utils method |
| src/test/java/cloud/eppo/UtilsTest.java | Added comprehensive tests for custom codec injection, default behavior, null handling, and test isolation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
aarsilv
left a comment
There was a problem hiding this comment.
Nice! Approving with minor comments
| "zero byte output from Base64; if not running on Android hardware be sure to use" | ||
| + " RobolectricTestRunner"); |
There was a problem hiding this comment.
I wonder if we need something similar for base64Encode
There was a problem hiding this comment.
🤔 Looking a little wider at it, we're only actually using the encode method in tests. I'd prefer to keep the Codec here just in case prod code needs to encode in the future we don't have to make a breaking change to add it.
| public class UtilsTest { | ||
|
|
||
| @AfterEach | ||
| void resetCodec() { |
There was a problem hiding this comment.
any reason these are package level instead of class? (e.g., why no public keyword here?)
There was a problem hiding this comment.
package-private is enough for JUnit to find them (uses reflection/annotation to find the methods)
| assertEquals("encoded:test", Utils.base64Encode("test")); | ||
| assertTrue(encodeCalled.get()); | ||
|
|
||
| assertEquals("decoded:test", Utils.base64Decode("test")); | ||
| assertTrue(decodeCalled.get()); |
6c4391b to
54cfb3c
Compare
* Configuration Source Extraction part 1/4 - Extract DTO Interfaces (#197) Convert all concrete DTO classes from cloud.eppo.ufc.dto to interfaces in cloud.eppo.api.dto with nested Default implementation classes. Converted classes: - Allocation, BanditFlagVariation, Shard, TargetingCondition, TargetingRule, Split - BanditAttributeCoefficients, BanditNumericAttributeCoefficients, BanditCategoricalAttributeCoefficients - BanditCoefficients, BanditModelData, BanditParameters, BanditParametersResponse - BanditReference, FlagConfig, FlagConfigResponse, Variation The ufc.dto package now only contains the adapters subdirectory. * feat: nullability annotations on DTOs (#218) * nullability for DTOs * drop setters * refine nullability * chore: move bandit attribute scoring out of DTO (#219) * chore: move business logic out of DTO * lint * [config-source] Add flagsSnapshotId Field to Congifuration (#208) * feat: add flagsSnapshotId field to Configuration * lint * [config source] HTTP Client and Parser Interfaces (#213) * feat: request, response, client, factory * return BanditParamsResponse from the parser and create a config builder to take same * drop serialize methods * update javadocs * lint * Add nullability annotations to HTTP and parser interfaces - EppoConfigurationRequest: @NotNull on getters - EppoConfigurationRequestFactory: @NotNull on factory method returns - EppoConfigurationResponse: @Nullable/@NotNull on constructor, factory methods, and getters - ConfigurationParser: @NotNull on method params and returns - EppoConfigurationClient: @NotNull on get() method * lint * Add json value unwrapping to Parser interface * [config source] - eppo-sdk-common Module (#214) * common module extraction with default parser and http client * build deps * [config-source] Integrate and test parser/config client (#215) * common module extraction with default parser and http client * build deps * adust to parser interface * Optionally use ConfigurationParser and ConfigurationRequestClients in the ConfigurationRequestor * Cut over to real implementations in testing * Common client (okhttp and Jackson for parsing) * use config.flagsSnapshotId * [config-source] Cleanup (#217) * Hard cut to ConfigurationClient * remove deprecated http client * hard cut to ConfigurationParser * remove grafting format field * drop serialize methods * rip mapper from configuration * parameterize the Json Flag type on the client * remove the response bytes from the Configuration data object * removed unused annotations * cut over jackson * update config docs * remove jackson from bandit action attributes * re-delegate json parsing/unwrapping * move okhttp and jackson to test only * feat: add pluggable Base64Codec interface for Android compatibility (#223) * feat(utils): add Base64Codec interface for pluggable encoding Add pluggable Base64 codec support to allow platform-specific implementations (e.g., Android SDK using android.util.Base64). - Add public Base64Codec interface with encode/decode methods - Add setBase64Codec() method for custom codec injection - Create DefaultBase64Codec inner class using java.util.Base64 - Delegate existing base64Encode/base64Decode methods to codec * refactor(deserializer): use Utils.base64Decode instead of duplicate Remove duplicate base64Decode implementation from FlagConfigResponseDeserializer and use the centralized version from Utils via static import. * test(utils): add tests for Base64Codec pluggability Add tests to verify Base64 codec pluggability: - testCustomBase64Codec: verifies custom codec injection works - testBase64EncodeDecodeDefault: verifies default codec behavior - @AfterEach reset method ensures test isolation * fix(utils): add volatile, null check, reset method, and UTF-8 charset - Add volatile keyword to base64Codec field for thread safety - Add null check in setBase64Codec() throwing IllegalArgumentException - Add resetBase64Codec() method for test cleanup - Use explicit StandardCharsets.UTF_8 in base64Decode - Add test for null codec rejection - Simplify test reset to use resetBase64Codec method * make resetCodec package private * fix: enable publishing for both sdk-common-jvm and eppo-sdk-framework artifacts (#224) * publish snapshot on pushes to snapshot branches * deploy common and framework modules * feat(http): extend EppoConfigurationRequest to support POST with request body (#225) * feat(http): add POST request support to configuration client Add support for POST requests with request body to EppoConfigurationClient: - Add HttpMethod enum and body/contentType fields to EppoConfigurationRequest - Rename EppoConfigurationClient.get() to execute() (with backward-compatible deprecated get()) - Update OkHttpEppoClient to handle POST requests with body - Update ConfigurationRequestor to use execute() method - Update BaseEppoClient initialization to pass new dependencies - Add comprehensive tests for POST request handling This enables the SDK to send configuration requests with POST method and request body when needed, while maintaining backward compatibility with existing GET-based requests. * Add Serializable support to Configuration and DTOs (#226) * Add Serializable to Configuration and DTOs * Add nullability annotations and immutability to DTOs * feat: common root class for serializable configurations * Use transitive dependency for framework instead of source bundling (#229) * Inline framework sources in eppo-sdk-common instead of project dependency - Add root src/main/java and resources to common module sourceSets - Replace api project(':') with framework lib deps (annotations, java-semver, commons-collections4) - Use withSourcesJar/withJavadocJar; remove custom jar/sourcesJar/javadocJar and POM stripping - Apply Spotless via apply plugin; limit java target to this module's src/main/java - Set sourcesJar duplicatesStrategy to EXCLUDE for multi-dir packaging Made-with: Cursor * Use transitive dependency instead of source bundling for framework Replace source inlining approach with standard Gradle transitive dependency (api project(':')). The framework is now properly declared as a dependency rather than having its sources compiled directly into the common module. Changes: - Remove sourceSets configuration that inlined framework sources - Add api project(':') dependency to pull in framework transitively - Remove explicit framework library deps (now inherited transitively) - Add versionMapping for proper dependency resolution - Add explicit POM configuration to ensure framework dependency appears in generated POM for Maven compatibility The Gradle module metadata (.module file) automatically includes the framework dependency correctly, but Maven POM generation requires explicit configuration via withXml. Result: Cleaner architecture with proper module separation and standard dependency management. * clean up packaging * update build comment * Configure archivesName to improve Gradle module metadata dependency resolution Set base.archivesName='eppo-sdk-framework' in root build.gradle to fix the mismatch between rootProject.name ('sdk-common-jvm') and the published artifactId ('eppo-sdk-framework'). This allows Gradle to correctly map 'api project(':')' references to Maven coordinates in Gradle module metadata (.module files). Maven POM generation still requires the withXml workaround due to a known Gradle limitation with root project references. Updated comments to clarify that: - Gradle module metadata works correctly with archivesName configured - withXml block is specifically needed for Maven POM consumers - This is a known Gradle limitation (gradle/gradle#16784) * fix: spotless in module * fix: feature/v4 bug surfaced by review (#237) * fix: orphaned futures, and thread-safety
Motivation and Context
java.util.Base64is only available on Android API 26+ (Android 8.0 Oreo). The Android SDK needs to useandroid.util.Base64for compatibility with older devices. This change introduces a pluggable Base64 codec interface that allows platform-specific implementations.Reference: Eppo-exp/android-sdk#188
Description
Base64Codecpublic interface inUtils.javawithbase64Encode(String)andbase64Decode(String)methodssetBase64Codec(Base64Codec)static method to allow consumers to inject custom implementationsDefaultBase64Codecprivate inner class that usesjava.util.Base64(existing behavior)base64Encode()andbase64Decode()static methods to delegate to the codec instancebase64Decodeimplementation fromFlagConfigResponseDeserializer.javaHow has this been documented?
How has this been tested?
testCustomBase64Codec()to verify custom codec injection works correctlytestBase64EncodeDecodeDefault()to verify default codec behavior including null handling and round-trip encoding@AfterEachreset method to ensure test isolation./gradlew spotlessApplyand./gradlew checksuccessfully