diff --git a/.claude/tasks/fix-plan.md b/.claude/tasks/fix-plan.md new file mode 100644 index 0000000..3d19c50 --- /dev/null +++ b/.claude/tasks/fix-plan.md @@ -0,0 +1,203 @@ +# Java SDK Fix Plan + +Based on full review of PR #11. + +## Critical + +### 1. Fix MatchOperator unbounded thread pool +- **File**: `core-api/src/main/java/com/absmartly/sdk/jsonexpr/operators/MatchOperator.java:17` +- **Issue**: `Executors.newCachedThreadPool()` is static, never shut down, and unbounded. Can exhaust threads under load. +- **Fix**: Replace with a bounded thread pool: + ```java + private static final ExecutorService REGEX_POOL = new ThreadPoolExecutor( + 0, 4, 60L, TimeUnit.SECONDS, new SynchronousQueue<>(), + new ThreadPoolExecutor.CallerRunsPolicy()); + ``` + Or better: use `Thread.interrupt()` with a timeout on a single thread, or use Java's `Pattern.compile` with input length limits instead of a thread pool. + +## Important + +### 2. Use JsonMapperUtils in DefaultContextEventSerializer +- **File**: `core-api/src/main/java/com/absmartly/sdk/DefaultContextEventSerializer.java:18` +- **Fix**: Replace `new ObjectMapper()` with `JsonMapperUtils.createStandardObjectMapper()` for consistency. + +### 3. Normalize endpoint URL trailing slash +- **File**: `core-api/src/main/java/com/absmartly/sdk/Client.java:59` +- **Fix**: Strip trailing slash from endpoint in constructor: + ```java + this.endpoint = endpoint.endsWith("/") ? endpoint.substring(0, endpoint.length() - 1) : endpoint; + ``` + +### 4. Fix MatchOperator Java 7/Android compatibility +- **File**: `core-api/src/main/java/com/absmartly/sdk/jsonexpr/operators/MatchOperator.java:17-21` +- **Issue**: Uses lambda expressions while rest of codebase uses anonymous inner classes for Java 7 compatibility. +- **Fix**: Convert lambdas to anonymous inner classes, or document that minimum Java version is now 8+. + +## Minor + +### 5. Add @deprecated Javadoc to deprecated wrappers +- **Files**: `core-api/src/main/java/com/absmartly/sdk/deprecated/ABSmartly.java`, `ABSmartlyConfig.java` +- **Fix**: Add `@deprecated Use {@link com.absmartly.sdk.ABsmartly} instead` Javadoc tags. + +### 6. Standardize validation error messages +- **Files**: `ABsmartly.java:59-62`, `Client.java` +- **Fix**: Use consistent wording across builder and client validation. + +### 7. Fix ABsmartly.close() thread safety +- **File**: `core-api/src/main/java/com/absmartly/sdk/ABsmartly.java:137-151` +- **Fix**: Add a `volatile boolean closed` flag checked at the start of `createContext()`, or synchronize `close()`. + +### 8. Fix branding in error messages +- **File**: `core-api/src/main/java/com/absmartly/sdk/Context.java:704-706` +- **Fix**: Change "ABSmartly Context" to "ABsmartly Context". + +## Additional Findings (Full Review v2) + +### Security + +#### 9. Client.java uses System.err for HTTP warning instead of SLF4J +- **File**: `core-api/src/main/java/com/absmartly/sdk/Client.java:36-38` +- **Issue**: `System.err.println()` is used for the non-HTTPS warning. The rest of the codebase uses SLF4J. This bypasses log configuration and won't appear in structured logs. +- **Fix**: Replace with `log.warn(...)` using an SLF4J logger, consistent with the rest of the SDK. + +#### 10. MatchOperator regex timeout cancel(true) does not actually interrupt regex +- **File**: `core-api/src/main/java/com/absmartly/sdk/jsonexpr/operators/MatchOperator.java:48` +- **Issue**: `future.cancel(true)` sends an interrupt to the thread, but `Pattern.matcher().find()` does not check the interrupt flag — the regex engine will continue running until completion. The timeout only prevents the *caller* from waiting, but the thread pool thread remains busy. Under sustained ReDoS, threads accumulate since the unbounded `newCachedThreadPool` keeps spawning new ones. +- **Severity**: This compounds with finding #1 (unbounded pool). Even with a bounded pool, stuck regex threads exhaust the pool. +- **Fix**: Consider using `CharSequence` wrapper that throws on `charAt()` when interrupted (common ReDoS mitigation pattern), or limit input text length in addition to pattern length. + +### Code Quality + +#### 11. EqualsOperator no longer extends BinaryOperator — inconsistency +- **File**: `core-api/src/main/java/com/absmartly/sdk/jsonexpr/operators/EqualsOperator.java` +- **Issue**: `EqualsOperator` was changed from `extends BinaryOperator` to `implements Operator` to add null==null handling. All other binary operators still extend `BinaryOperator`. This breaks the consistent operator hierarchy pattern and duplicates the argument extraction logic from `BinaryOperator.evaluate()`. +- **Fix**: Either add null-null handling to `BinaryOperator` base class (with an override hook), or document why `EqualsOperator` needs to be special. + +#### 12. InOperator argument order swap is a breaking semantic change +- **File**: `core-api/src/main/java/com/absmartly/sdk/jsonexpr/operators/InOperator.java:10` +- **Issue**: The `binary()` method signature changed from `binary(evaluator, haystack, needle)` to `binary(evaluator, needle, haystack)`. This is called by `BinaryOperator.evaluate()` which passes `(lhs, rhs)` — so the semantic meaning of the arguments in the audience JSON `["in", needle, haystack]` changed. The cross-SDK tests pass, confirming this is the correct order, but the original code had a bug where the arguments were swapped. This is fine as a bug fix but should be noted. +- **Severity**: Low — correctly fixed to match cross-SDK specification. + +#### 13. Builder does not expose all ABsmartlyConfig options +- **File**: `core-api/src/main/java/com/absmartly/sdk/ABsmartly.java:24-78` +- **Issue**: The `Builder` only exposes `endpoint`, `apiKey`, `application`, `environment`, and `eventLogger`. It doesn't expose `scheduler`, `variableParser`, `audienceDeserializer`, `contextDataProvider`, `contextEventHandler`, or `httpClient` options — all of which are documented in the README's SDK Options table. Users needing these must fall back to the config-based API. +- **Fix**: Either add these to the builder, or document that the builder is a quickstart convenience and advanced options require the config API. + +### Performance + +#### 14. Context.buildAttributesMap() allocates on every call without caching +- **File**: `core-api/src/main/java/com/absmartly/sdk/Context.java:1128-1133` +- **Issue**: `buildAttributesMap()` is called inside `getAssignment()` (hot path) and `audienceMatches()`. Each call creates a new `HashMap` and copies all attributes. With the `attrsSeq_` tracking, `audienceMatches()` is called on every non-cached assignment check. For contexts with many attributes being evaluated frequently, this adds GC pressure. +- **Severity**: Low — only matters under high throughput. +- **Fix**: Cache the map and invalidate when `attrsSeq_` changes. Or just note this as acceptable. + +#### 15. DefaultContextEventSerializer still uses plain ObjectMapper, not JsonMapperUtils +- **File**: `core-api/src/main/java/com/absmartly/sdk/DefaultContextEventSerializer.java:18` +- **Issue**: Same as existing finding #2 — still uses `new ObjectMapper()` instead of `JsonMapperUtils.createStandardObjectMapper()`. This means the serializer doesn't get `FAIL_ON_READING_DUP_TREE_KEY` or `USE_STATIC_TYPING` applied consistently. +- **Note**: Duplicate of finding #2, confirmed still present. + +### Correctness + +#### 16. Context.setData() NPE when experiment.variants is null +- **File**: `core-api/src/main/java/com/absmartly/sdk/Context.java:1008-1014` +- **Issue**: `setData()` iterates `experiment.variants` with a for-each loop. If the server returns an experiment with `variants: null` (which the deserialization tests show is possible — see `testNullFieldsInExperiment`), this throws `NullPointerException`. +- **Fix**: Add null guard: `if (experiment.variants != null)` before the for-each loop. + +#### 17. Context.setData() NPE when experiment.customFieldValues is null +- **File**: `core-api/src/main/java/com/absmartly/sdk/Context.java:1048-1060` +- **Issue**: Similar to #16. If `experiment.customFieldValues` is null, the `entrySet()` iteration will NPE. +- **Fix**: Add null guard before iterating custom field values. + +#### 18. ABsmartly.close() race condition between close() and createContext() +- **File**: `core-api/src/main/java/com/absmartly/sdk/ABsmartly.java:134-151` +- **Issue**: Same as existing finding #7. `close()` sets `client_ = null` and `scheduler_ = null` without synchronization. A concurrent `createContext()` call can see a null scheduler or use a closed client. The fields are not volatile either. +- **Note**: Duplicate of finding #7, confirmed still present. This is the most impactful correctness issue remaining. + +## Additional Findings (Full Review v3) + +### Security + +#### 19. MatchOperator lambda breaks Java 8 source compatibility claim AND leaks threads +- **File**: `core-api/src/main/java/com/absmartly/sdk/jsonexpr/operators/MatchOperator.java:17-21,40` +- **Issue**: The `MatchOperator` uses lambda expressions (`r -> { ... }` on line 17, `() -> { ... }` on line 40) while the build.gradle explicitly sets `sourceCompatibility = "1.8"` and `targetCompatibility = "1.8"`. Java 8 supports lambdas, so this compiles — but the rest of the codebase consistently uses anonymous inner classes (see Client.java, Context.java). This is a style inconsistency, not a compile error. However, the static `ExecutorService executor` field (line 17) is **never shut down**. When the JVM classloader unloads the class or in application server environments with redeployable WARs, this creates a thread leak. There is no shutdown hook or `close()` method. +- **Severity**: Important (thread leak in long-running app server environments) +- **Fix**: Either (a) add a static `shutdown()` method and document when to call it, or (b) use daemon threads (already done) and accept leak risk, or (c) replace the thread pool approach entirely with an interruptible CharSequence wrapper (see finding #10). +- **Status**: Partially overlaps with findings #1 and #4 but adds the classloader leak dimension. + +#### 20. AudienceMatcher creates byte[] from audience string on every evaluate() call +- **File**: `core-api/src/main/java/com/absmartly/sdk/AudienceMatcher.java:31` +- **Issue**: `audience.getBytes(StandardCharsets.UTF_8)` allocates a new byte array every call, then deserializes it. The same audience string is evaluated repeatedly for every assignment check. This is wasteful and creates GC pressure. +- **Severity**: Minor/Performance +- **Fix**: Consider caching the deserialized audience map keyed by the audience string, since audience strings don't change between refreshes. + +### Code Quality + +#### 21. Context.flush() acquires contextLock_.writeLock() for read-only operation +- **File**: `core-api/src/main/java/com/absmartly/sdk/Context.java:650-662` +- **Issue**: The `flush()` method acquires `contextLock_.writeLock()` (line 650) to read `units_` and `attributes_`. It doesn't modify either collection. This blocks all concurrent `getTreatment()`/`getVariableValue()` calls (which use `contextLock_.readLock()`) during the entire serialization of units. Under high throughput, a flush can stall all assignment reads. +- **Severity**: Important (performance under concurrency) +- **Fix**: Use `contextLock_.readLock()` instead. The `units_` map is only written in `setUnit()` (under write lock) and `attributes_` list is only appended to via `Concurrency.addRW()`. Reading them under read lock is safe. + +#### 22. ExprEvaluator.extractVar() splits path string on every call without caching +- **File**: `core-api/src/main/java/com/absmartly/sdk/jsonexpr/ExprEvaluator.java:95` +- **Issue**: `path.split("/")` is called every time `extractVar()` is evaluated. In audience expressions that reference variables (e.g., `{"var": "user/age"}`), this creates a new String array allocation per evaluation. The `split()` method also compiles a regex internally each time (though Java's `String.split` optimizes single-char patterns). +- **Severity**: Minor — negligible overhead for simple paths. + +#### 23. Context.getAssignment() builds new Assignment object on every cache miss without lock upgrade +- **File**: `core-api/src/main/java/com/absmartly/sdk/Context.java:763-885` +- **Issue**: The `getAssignment()` method first acquires a read lock (line 764), checks the cache, releases it (line 794), then acquires a write lock (line 798) and re-does all the lookups again. Between releasing the read lock and acquiring the write lock, another thread could have already populated the cache for the same experiment. This is a classic TOCTOU (time-of-check-time-of-use) pattern. While it doesn't cause incorrect behavior (the second assignment will just overwrite with an equivalent one), it wastes CPU on duplicate computation. +- **Severity**: Minor — correct but wasteful. Standard pattern for read-write lock upgrade. + +#### 24. Context.checkNotClosed() uses old "ABSmartly" branding +- **File**: `core-api/src/main/java/com/absmartly/sdk/Context.java:703-707` +- **Issue**: Error messages say "ABSmartly Context" (capital S) instead of "ABsmartly Context". Same as finding #8, confirming it's still present in the v3 review. +- **Status**: Duplicate of finding #8, confirmed still present. + +### Performance + +#### 25. HashMap initial capacity not optimized in Context constructor +- **File**: `core-api/src/main/java/com/absmartly/sdk/Context.java:58,65-66` +- **Issue**: `units_` is initialized as `new HashMap()` (default capacity 16), then `assigners_` and `hashedUnits_` are sized to `units_.size()` which at that point is 0. These maps will be resized as units are added. Since `units` from `config.getUnits()` is known at construction time, the initial capacity should be derived from it. +- **Severity**: Minor — negligible for typical unit counts (1-3). + +#### 26. Algorithm.mapSetToArray uses reflection to create array +- **File**: `core-api/src/main/java/com/absmartly/sdk/internal/Algorithm.java:11` +- **Issue**: `java.lang.reflect.Array.newInstance()` is used to create the output array. This is called during `flush()` to map units. While not a hot path (only called on publish), reflection-based array creation is slower than direct array allocation. +- **Severity**: Minor — only called during flush, not hot path. + +### Correctness + +#### 27. Context.setData() line 1013: NPE if experiment.variants is null (confirmed still present) +- **File**: `core-api/src/main/java/com/absmartly/sdk/Context.java:1013` +- **Issue**: `experiment.variants.length` will NPE if variants is null. The code at line 1057 already guards `experiment.customFieldValues != null`, but line 1013 does not guard `experiment.variants`. +- **Status**: Duplicate of finding #16, confirmed still present in v3 review. + +#### 28. Context.getCustomFieldValue() NPE when experiment has no custom field values map +- **File**: `core-api/src/main/java/com/absmartly/sdk/Context.java:215` +- **Issue**: `experiment.customFieldValues.get(key)` could NPE if `customFieldValues` is null. Looking at `setData()`, the `customFieldValues` map is always initialized (line 1056), so this is safe for experiments processed through `setData()`. However, if `setData()` is called with an experiment where `variants` is null (causing NPE at line 1013, finding #16), the experiment's `customFieldValues` map would never be initialized. This is a secondary failure mode of finding #16. +- **Severity**: Low — only reachable if finding #16 is triggered first. + +#### 29. Client.getContextData() empty response check may hide deserialization errors +- **File**: `core-api/src/main/java/com/absmartly/sdk/Client.java:99-101` +- **Issue**: When the response body is empty (`content == null || content.length == 0`), an `IllegalStateException` is thrown. However, if `deserializer_.deserialize()` returns null (which `DefaultContextDataDeserializer` does on IOException), the null is passed to `dataFuture.complete(null)`, which then propagates to `Context.setData(null)` which throws `IllegalArgumentException("Context data cannot be null")`. The error message at that point is misleading — it suggests a programming error rather than a deserialization failure. +- **Severity**: Minor — the error is caught, just misleading message. +- **Fix**: Add null check after deserialization: `if (result == null) dataFuture.completeExceptionally(...)`. + +### Simplification Opportunities + +#### 30. EqualsOperator could use BinaryOperator with null-aware override +- **File**: `core-api/src/main/java/com/absmartly/sdk/jsonexpr/operators/EqualsOperator.java` +- **Issue**: Same as finding #11. The EqualsOperator directly implements `Operator` and duplicates argument extraction from BinaryOperator. A cleaner approach would be to add a `nullableBinary()` method to BinaryOperator or an `allowNulls()` flag. +- **Status**: Duplicate of finding #11. + +#### 31. Context inner class Assignment could be simplified +- **File**: `core-api/src/main/java/com/absmartly/sdk/Context.java:742-761` +- **Issue**: The `Assignment` class has 13 mutable fields with no encapsulation. While this is internal, it would benefit from being a simple data holder with final fields set via constructor. However, this would be a larger refactor and the current approach works. +- **Severity**: Minor — internal class, not part of public API. + +### Summary of v3 Review + +**New findings**: #19-31 (13 items) +**Confirmed still present from v1/v2**: #1, #2, #4, #7/#18, #8/#24, #9, #10, #11/#30, #16/#27 +**Truly new actionable items**: #20 (audience byte[] allocation), #21 (write lock in flush), #29 (null deserialization propagation) +**Most impactful new finding**: #21 — Context.flush() unnecessarily acquires write lock, blocking all concurrent reads diff --git a/README.md b/README.md index dc2d045..1ba01c5 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,7 @@ If you target Android 6.0 or earlier, a few extra steps are outlined below for i #### Gradle -To install the ABSmartly SDK, place the following in your `build.gradle` and replace {VERSION} with the latest SDK version available in MavenCentral. +To install the ABsmartly SDK, place the following in your `build.gradle` and replace {VERSION} with the latest SDK version available in MavenCentral. ```gradle dependencies { @@ -33,7 +33,7 @@ dependencies { #### Maven -To install the ABSmartly SDK, place the following in your `pom.xml` and replace {VERSION} with the latest SDK version available in MavenCentral. +To install the ABsmartly SDK, place the following in your `pom.xml` and replace {VERSION} with the latest SDK version available in MavenCentral. ```xml @@ -47,8 +47,8 @@ To install the ABSmartly SDK, place the following in your `pom.xml` and replace When targeting Android 6.0 or earlier, the default Java Security Provider will not work. Using [Conscrypt](https://github.com/google/conscrypt) is recommended. Follow these [instructions](https://github.com/google/conscrypt/blob/master/README.md) to install it as dependency. #### Proguard rules -ProGuard is a command-line tool that reduces app size by shrinking bytecode and obfuscates the names of classes, fields and methods. -It’s an ideal fit for developers working with Java or Kotlin who are primarily interested in an Android optimizer. +ProGuard is a command-line tool that reduces app size by shrinking bytecode and obfuscates the names of classes, fields and methods. +It's an ideal fit for developers working with Java or Kotlin who are primarily interested in an Android optimizer. If you are using [Proguard](https://github.com/Guardsquare/proguard), you will need to add the following rule to your Proguard configuration file. This prevent proguard to change data classes used by the SDK and the missing of this rule will result in problems in the serialization/deserialization of the data. ```proguard @@ -59,32 +59,62 @@ This prevent proguard to change data classes used by the SDK and the missing of Please follow the [installation](#installation) instructions before trying the following code: -#### Initialization +### Initialization + This example assumes an Api Key, an Application, and an Environment have been created in the A/B Smartly web console. + +#### Quickstart + ```java import com.absmartly.sdk.*; -public class Example { - static public void main(String[] args) { +final ABsmartly sdk = ABsmartly.builder() + .endpoint("https://your-company.absmartly.io/v1") + .apiKey(System.getenv("ABSMARTLY_APIKEY")) + .application("website") + .environment("production") + .build(); +``` - final ClientConfig clientConfig = ClientConfig.create() - .setEndpoint("https://your-company.absmartly.io/v1") - .setAPIKey("YOUR-API-KEY") - .setApplication("website") // created in the ABSmartly web console - .setEnvironment("development"); // created in the ABSmartly web console +The builder pattern lets you configure the SDK with named parameters, removing the need to configure `ClientConfig` and `ABsmartlyConfig` manually. - final Client absmartlyClient = Client.create(clientConfig); +#### Alternative: Using Configuration Objects - final ABSmartlyConfig sdkConfig = ABSmartlyConfig.create() - .setClient(absmartlyClient); +For use cases where you need full control over the Client and configuration: +```java +final ClientConfig clientConfig = ClientConfig.create() + .setEndpoint("https://your-company.absmartly.io/v1") + .setAPIKey("YOUR-API-KEY") + .setApplication("website") + .setEnvironment("development"); - final ABSmartly sdk = ABSmartly.create(sdkConfig); - // ... - } -} +final Client absmartlyClient = Client.create(clientConfig); + +final ABsmartlyConfig sdkConfig = ABsmartlyConfig.create() + .setClient(absmartlyClient); + +final ABsmartly sdk = ABsmartly.create(sdkConfig); ``` +**SDK Options** + +| Config | Type | Required? | Default | Description | +| :---------------------- | :-------------------------------- | :-------: | :---------: | :---------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| endpoint | `String` | ✅ | `null` | The URL to your API endpoint. Most commonly `"https://your-company.absmartly.io/v1"` | +| apiKey | `String` | ✅ | `null` | Your API key which can be found on the Web Console. | +| environment | `String` | ✅ | `null` | The environment of the platform where the SDK is installed. Environments are created on the Web Console and should match the available environments in your infrastructure. | +| application | `String` | ✅ | `null` | The name of the application where the SDK is installed. Applications are created on the Web Console and should match the applications where your experiments will be running. | +| timeout | `int` | ❌ | `3000` | HTTP connection timeout in milliseconds | +| retries | `int` | ❌ | `5` | Maximum number of retry attempts for failed HTTP requests | +| contextEventLogger | `ContextEventLogger` | ❌ | `null` | Callback to handle SDK events (ready, exposure, goal, etc.) | +| contextDataProvider | `ContextDataProvider` | ❌ | auto | Custom provider for context data (advanced usage) | +| contextEventHandler | `ContextEventHandler` | ❌ | auto | Custom handler for publishing events (advanced usage) | +| variableParser | `VariableParser` | ❌ | auto | Custom parser for experiment variables (advanced usage) | +| audienceDeserializer | `AudienceDeserializer` | ❌ | auto | Custom deserializer for audience data (advanced usage) | +| scheduler | `ScheduledExecutorService` | ❌ | auto | Custom scheduler for context refresh (advanced usage) | +| httpClient | `HTTPClient` | ❌ | auto | Custom HTTP client implementation (advanced usage) | + #### Android 6.0 or earlier When targeting Android 6.0 or earlier, set the default Java Security Provider for SSL to *Conscrypt* by creating the *Client* instance as follows: @@ -96,8 +126,8 @@ import org.conscrypt.Conscrypt; final ClientConfig clientConfig = ClientConfig.create() .setEndpoint("https://your-company.absmartly.io/v1") .setAPIKey("YOUR-API-KEY") - .setApplication("website") // created in the ABSmartly web console - .setEnvironment("development"); // created in the ABSmartly web console + .setApplication("website") + .setEnvironment("development"); final DefaultHTTPClientConfig httpClientConfig = DefaultHTTPClientConfig.create() .setSecurityProvider(Conscrypt.newProvider()); @@ -106,53 +136,76 @@ import org.conscrypt.Conscrypt; final Client absmartlyClient = Client.create(clientConfig, httpClient); - final ABSmartlyConfig sdkConfig = ABSmartlyConfig.create() + final ABsmartlyConfig sdkConfig = ABsmartlyConfig.create() .setClient(absmartlyClient); - final ABSmartly sdk = ABSmartly.create(sdkConfig); + final ABsmartly sdk = ABsmartly.create(sdkConfig); // ... ``` -#### Creating a new Context synchronously +## Creating a New Context + +### Synchronously + ```java -// define a new context request - final ContextConfig contextConfig = ContextConfig.create() - .setUnit("session_id", "5ebf06d8cb5d8137290c4abb64155584fbdb64d8"); // a unique id identifying the user +final ContextConfig contextConfig = ContextConfig.create() + .setUnit("session_id", "5ebf06d8cb5d8137290c4abb64155584fbdb64d8"); - final Context context = sdk.createContext(contextConfig) - .waitUntilReady(); +final Context context = sdk.createContext(contextConfig) + .waitUntilReady(); ``` -#### Creating a new Context asynchronously +### Asynchronously + ```java -// define a new context request - final ContextConfig contextConfig = ContextConfig.create() - .setUnit("session_id", "5ebf06d8cb5d8137290c4abb64155584fbdb64d8"); // a unique id identifying the user +final ContextConfig contextConfig = ContextConfig.create() + .setUnit("session_id", "5ebf06d8cb5d8137290c4abb64155584fbdb64d8"); - final Context context = sdk.createContext(contextConfig) - .waitUntilReadyAsync() - .thenAccept(ctx -> System.out.printf("context ready!")); +final Context context = sdk.createContext(contextConfig) + .waitUntilReadyAsync() + .thenAccept(ctx -> System.out.printf("context ready!")); ``` -#### Creating a new Context with pre-fetched data +### With Pre-fetched Data + Creating a context involves a round-trip to the A/B Smartly event collector. We can avoid repeating the round-trip on the client-side by re-using data previously retrieved. ```java - final ContextConfig contextConfig = ContextConfig.create() - .setUnit("session_id", "5ebf06d8cb5d8137290c4abb64155584fbdb64d8"); // a unique id identifying the user +final ContextConfig contextConfig = ContextConfig.create() + .setUnit("session_id", "5ebf06d8cb5d8137290c4abb64155584fbdb64d8"); + +final Context context = sdk.createContext(contextConfig) + .waitUntilReady(); + +final ContextConfig anotherContextConfig = ContextConfig.create() + .setUnit("session_id", "5ebf06d8cb5d8137290c4abb64155584fbdb64d8"); + +final Context anotherContext = sdk.createContextWith(anotherContextConfig, context.getData()); +assert(anotherContext.isReady()); // no need to wait +``` + +### Refreshing the Context with Fresh Experiment Data - final Context context = sdk.createContext(contextConfig) - .waitUntilReady(); +For long-running contexts, the context is usually created once when the application is first started. +However, any experiments being tracked in your production code, but started after the context was created, will not be triggered. +To mitigate this, we can use the `setRefreshInterval()` method on the context config. + +```java +final ContextConfig contextConfig = ContextConfig.create() + .setUnit("session_id", "5ebf06d8cb5d8137290c4abb64155584fbdb64d8") + .setRefreshInterval(TimeUnit.HOURS.toMillis(4)); // every 4 hours +``` - final ContextConfig anotherContextConfig = ContextConfig.create() - .setUnit("session_id", "5ebf06d8cb5d8137290c4abb64155584fbdb64d8"); // a unique id identifying the other user +Alternatively, the `refresh()` method can be called manually. +The `refresh()` method pulls updated experiment data from the A/B Smartly collector and will trigger recently started experiments when `getTreatment()` is called again. - final Context anotherContext = sdk.createContextWith(anotherContextConfig, context.getData()); - assert(anotherContext.isReady()); // no need to wait +```java +context.refresh(); ``` -#### Setting extra units for a context +### Setting Extra Units + You can add additional units to a context by calling the `setUnit()` or the `setUnits()` method. This method may be used for example, when a user logs in to your application, and you want to use the new unit type to the context. Please note that **you cannot override an already set unit type** as that would be a change of identity, and will throw an exception. In this case, you must create a new context instead. @@ -162,162 +215,548 @@ The `setUnit()` and `setUnits()` methods can be called before the context is rea context.setUnit("db_user_id", "1000013"); context.setUnits(Map.of( - "db_user_id", "1000013" - )); + "db_user_id", "1000013" +)); ``` -#### Setting context attributes -The `setAttribute()` and `setAttributes()` methods can be called before the context is ready. +## Basic Usage + +### Selecting a Treatment + ```java - context.setAttribute('user_agent', req.getHeader("User-Agent")); +if (context.getTreatment("exp_test_experiment") == 0) { + // user is in control group (variant 0) +} else { + // user is in treatment group +} +``` + +### Treatment Variables + +```java +final Object variable = context.getVariable("my_variable"); +``` - context.setAttributes(Map.of( - "customer_age", "new_customer" - )); +### Peek at Treatment Variants + +Although generally not recommended, it is sometimes necessary to peek at a treatment or variable without triggering an exposure. +The A/B Smartly SDK provides a `peekTreatment()` method for that. + +```java +if (context.peekTreatment("exp_test_experiment") == 0) { + // user is in control group (variant 0) +} else { + // user is in treatment group +} ``` -#### Selecting a treatment +#### Peeking at Variables + ```java - if (context.getTreament("exp_test_experiment") == 0) { - // user is in control group (variant 0) - } else { - // user is in treatment group - } +final Object variable = context.peekVariable("my_variable"); ``` -#### Selecting a treatment variable +### Overriding Treatment Variants + +During development, for example, it is useful to force a treatment for an experiment. This can be achieved with the `setOverride()` and/or `setOverrides()` methods. +The `setOverride()` and `setOverrides()` methods can be called before the context is ready. + ```java - final Object variable = context.getVariable("my_variable"); +context.setOverride("exp_test_experiment", 1); +context.setOverrides(Map.of( + "exp_test_experiment", 1, + "exp_another_experiment", 0 +)); ``` -#### Tracking a goal achievement +## Advanced + +### Context Attributes + +The `setAttribute()` and `setAttributes()` methods can be called before the context is ready. + +```java +context.setAttribute("user_agent", req.getHeader("User-Agent")); + +context.setAttributes(Map.of( + "customer_age", "new_customer" +)); +``` + +### Tracking Goals + Goals are created in the A/B Smartly web console. + ```java - context.track("payment", Map.of( - "item_count", 1, - "total_amount", 1999.99 - )); +context.track("payment", Map.of( + "item_count", 1, + "total_amount", 1999.99 +)); ``` -#### Publishing pending data +### Publishing Pending Data + Sometimes it is necessary to ensure all events have been published to the A/B Smartly collector, before proceeding. You can explicitly call the `publish()` or `publishAsync()` methods. + ```java - context.publish(); +context.publish(); ``` -#### Finalizing +### Finalizing + The `close()` and `closeAsync()` methods will ensure all events have been published to the A/B Smartly collector, like `publish()`, and will also "seal" the context, throwing an error if any method that could generate an event is called. + ```java - context.close(); +context.close(); ``` -#### Refreshing the context with fresh experiment data -For long-running contexts, the context is usually created once when the application is first started. -However, any experiments being tracked in your production code, but started after the context was created, will not be triggered. -To mitigate this, we can use the `setRefreshInterval()` method on the context config. +### Custom Event Logger + +The A/B Smartly SDK can be instantiated with an event logger used for all contexts. +In addition, an event logger can be specified when creating a particular context, in the `ContextConfig`. ```java - final ContextConfig contextConfig = ContextConfig.create() - .setUnit("session_id", "5ebf06d8cb5d8137290c4abb64155584fbdb64d8") - .setRefreshInterval(TimeUnit.HOURS.toMillis(4)); // every 4 hours +public class CustomEventLogger implements ContextEventLogger { + @Override + public void handleEvent(Context context, ContextEventLogger.EventType event, Object data) { + switch (event) { + case Exposure: + final Exposure exposure = (Exposure)data; + System.out.printf("exposed to experiment %s", exposure.name); + break; + case Goal: + final GoalAchievement goal = (GoalAchievement)data; + System.out.printf("goal tracked: %s", goal.name); + break; + case Error: + System.out.printf("error: %s", data); + break; + case Publish: + case Ready: + case Refresh: + case Close: + break; + } + } +} ``` -Alternatively, the `refresh()` method can be called manually. -The `refresh()` method pulls updated experiment data from the A/B Smartly collector and will trigger recently started experiments when `getTreatment()` is called again. +Usage: + ```java - context.refresh(); +// For all contexts, during SDK initialization +final ABsmartlyConfig sdkConfig = ABsmartlyConfig.create(); +sdkConfig.setContextEventLogger(new CustomEventLogger()); + +// OR, alternatively, during a particular context initialization +final ContextConfig contextConfig = ContextConfig.create(); +contextConfig.setEventLogger(new CustomEventLogger()); ``` -#### Using a custom Event Logger -The A/B Smartly SDK can be instantiated with an event logger used for all contexts. -In addition, an event logger can be specified when creating a particular context, in the `ContextConfig`. +**Event Types** + +| Event | When | Data | +| ---------- | ---------------------------------------------------------- | -------------------------------------- | +| `Error` | `Context` receives an error | `Throwable` object | +| `Ready` | `Context` turns ready | `ContextData` used to initialize | +| `Refresh` | `Context.refresh()` method succeeds | `ContextData` used to refresh | +| `Publish` | `Context.publish()` method succeeds | `PublishEvent` sent to collector | +| `Exposure` | `Context.getTreatment()` succeeds on first exposure | `Exposure` enqueued for publishing | +| `Goal` | `Context.track()` method succeeds | `GoalAchievement` enqueued for publishing | +| `Close` | `Context.close()` method succeeds the first time | `null` | + +## Platform-Specific Examples + +### Using with Spring Boot + ```java - // example implementation - public class CustomEventLogger implements ContextEventLogger { - @Override - public void handleEvent(Context context, ContextEventLogger.EventType event, Object data) { - switch (event) { - case Exposure: - final Exposure exposure = (Exposure)data; - System.out.printf("exposed to experiment %s", exposure.name); - break; - case Goal: - final GoalAchievement goal = (GoalAchievement)data; - System.out.printf("goal tracked: %s", goal.name); - break; - case Error: - System.out.printf("error: %s", data); - break; - case Publish: - case Ready: - case Refresh: - case Close: - break; - } +// Application.java +import com.absmartly.sdk.*; +import org.springframework.boot.SpringApplication; +import org.springframework.boot.autoconfigure.SpringBootApplication; +import org.springframework.context.annotation.Bean; +import org.springframework.beans.factory.annotation.Value; + +@SpringBootApplication +public class Application { + + @Value("${absmartly.endpoint}") + private String endpoint; + + @Value("${absmartly.apiKey}") + private String apiKey; + + @Value("${absmartly.application}") + private String application; + + @Value("${absmartly.environment}") + private String environment; + + @Bean + public ABsmartly absmartly() { + final ClientConfig clientConfig = ClientConfig.create() + .setEndpoint(endpoint) + .setAPIKey(apiKey) + .setApplication(application) + .setEnvironment(environment); + + final Client client = Client.create(clientConfig); + + final ABsmartlyConfig sdkConfig = ABsmartlyConfig.create() + .setClient(client); + + return ABsmartly.create(sdkConfig); + } + + public static void main(String[] args) { + SpringApplication.run(Application.class, args); + } +} + +// application.properties +absmartly.endpoint=https://your-company.absmartly.io/v1 +absmartly.apiKey=YOUR-API-KEY +absmartly.application=website +absmartly.environment=production + +// ProductController.java +import com.absmartly.sdk.*; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Controller; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.servlet.ModelAndView; +import jakarta.servlet.http.HttpSession; + +@Controller +public class ProductController { + + @Autowired + private ABsmartly absmartly; + + @GetMapping("/product") + public ModelAndView showProduct(HttpSession session) { + final ContextConfig contextConfig = ContextConfig.create() + .setUnit("session_id", session.getId()); + + final Context context = absmartly.createContext(contextConfig) + .waitUntilReady(); + + final int treatment = context.getTreatment("exp_product_layout"); + + context.close(); + + ModelAndView mav = new ModelAndView(); + if (treatment == 0) { + mav.setViewName("product_control"); + } else { + mav.setViewName("product_treatment"); } + + return mav; } +} ``` +### Using with Jakarta EE / JAX-RS + ```java - // for all contexts, during sdk initialization - final ABSmartlyConfig sdkConfig = ABSmartlyConfig.create(); - sdkConfig.setContextEventLogger(new CustomEventLogger()); - - // OR, alternatively, during a particular context initialization - final ContextConfig contextConfig = ContextConfig.create(); - contextConfig.setEventLogger(new CustomEventLogger()); -``` +// ABSmartlyProducer.java +import com.absmartly.sdk.*; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.enterprise.inject.Produces; -The data parameter depends on the type of event. -Currently, the SDK logs the following events: +@ApplicationScoped +public class ABSmartlyProducer { -| event | when | data | -|:---: |------------------------------------------------------------|---| -| `Error` | `Context` receives an error | `Throwable` object | -| `Ready` | `Context` turns ready | `ContextData` used to initialize the context | -| `Refresh` | `Context.refresh()` method succeeds | `ContextData` used to refresh the context | -| `Publish` | `Context.publish()` method succeeds | `PublishEvent` sent to the A/B Smartly event collector | -| `Exposure` | `Context.getTreatment()` method succeeds on first exposure | `Exposure` enqueued for publishing | -| `Goal` | `Context.track()` method succeeds | `GoalAchievement` enqueued for publishing | -| `Close` | `Context.close()` method succeeds the first time | `null` | + @Produces + @ApplicationScoped + public ABsmartly produceABSmartly() { + final ClientConfig clientConfig = ClientConfig.create() + .setEndpoint(System.getenv("ABSMARTLY_ENDPOINT")) + .setAPIKey(System.getenv("ABSMARTLY_API_KEY")) + .setApplication("website") + .setEnvironment(System.getenv("ENV")); + final Client client = Client.create(clientConfig); -#### Peek at treatment variants -Although generally not recommended, it is sometimes necessary to peek at a treatment or variable without triggering an exposure. -The A/B Smartly SDK provides a `peekTreatment()` method for that. + final ABsmartlyConfig sdkConfig = ABsmartlyConfig.create() + .setClient(client); + + return ABsmartly.create(sdkConfig); + } +} + +// ProductResource.java +import com.absmartly.sdk.*; +import jakarta.inject.Inject; +import jakarta.ws.rs.GET; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.core.Response; +import jakarta.servlet.http.HttpServletRequest; + +@Path("/product") +public class ProductResource { + + @Inject + private ABsmartly absmartly; + + @Inject + private HttpServletRequest request; + + @GET + public Response getProduct() { + final String sessionId = request.getSession().getId(); + + final ContextConfig contextConfig = ContextConfig.create() + .setUnit("session_id", sessionId); + + final Context context = absmartly.createContext(contextConfig) + .waitUntilReady(); + + final int treatment = context.getTreatment("exp_product_layout"); + + context.close(); + + return Response.ok() + .entity(Map.of("treatment", treatment)) + .build(); + } +} +``` + +### Using with Android Activities ```java - if (context.peekTreatment("exp_test_experiment") == 0) { - // user is in control group (variant 0) - } else { - // user is in treatment group +// MainActivity.java +import android.os.Bundle; +import androidx.appcompat.app.AppCompatActivity; +import com.absmartly.sdk.*; +import java.util.UUID; + +public class MainActivity extends AppCompatActivity { + + private static ABsmartly sdk; + private Context absmartlyContext; + + @Override + protected void onCreate(Bundle savedInstanceState) { + super.onCreate(savedInstanceState); + + // Initialize SDK once (typically in Application class) + if (sdk == null) { + final ClientConfig clientConfig = ClientConfig.create() + .setEndpoint("https://your-company.absmartly.io/v1") + .setAPIKey("YOUR-API-KEY") + .setApplication("android-app") + .setEnvironment("production"); + + final Client client = Client.create(clientConfig); + + final ABsmartlyConfig sdkConfig = ABsmartlyConfig.create() + .setClient(client); + + sdk = ABsmartly.create(sdkConfig); + } + + // Create context for this user + String deviceId = getDeviceId(); // Get from SharedPreferences + + final ContextConfig contextConfig = ContextConfig.create() + .setUnit("device_id", deviceId); + + final Context contextInstance = sdk.createContext(contextConfig); + absmartlyContext = contextInstance; + + contextInstance.waitUntilReadyAsync() + .thenAccept(ctx -> { + runOnUiThread(() -> setupUI(ctx)); + }) + .exceptionally(throwable -> { + runOnUiThread(() -> setupUIWithDefault()); + return null; + }); } + + private void setupUI(Context context) { + int treatment = context.getTreatment("exp_button_color"); + + if (treatment == 0) { + setContentView(R.layout.activity_main_control); + } else { + setContentView(R.layout.activity_main_treatment); + } + } + + private void setupUIWithDefault() { + setContentView(R.layout.activity_main_control); + } + + @Override + protected void onDestroy() { + super.onDestroy(); + if (absmartlyContext != null) { + absmartlyContext.close(); + } + } + + private String getDeviceId() { + // IMPORTANT: Device ID must be persisted across app sessions in SharedPreferences + // to ensure consistent experiment assignments for the same user/device. + // This example uses a random UUID for demonstration purposes only. + return UUID.randomUUID().toString(); + } +} ``` -##### Peeking at variables +## Advanced Request Configuration + +### HTTP Request Timeout Override + +Configure timeout for individual requests using DefaultHTTPClientConfig: + ```java - final Object variable = context.peekVariable("my_variable"); +import com.absmartly.sdk.*; + +// Create HTTP client with custom timeout +final DefaultHTTPClientConfig httpClientConfig = DefaultHTTPClientConfig.create() + .setConnectTimeout(1500) // 1.5 seconds + .setConnectionRequestTimeout(1500); + +final DefaultHTTPClient httpClient = DefaultHTTPClient.create(httpClientConfig); + +final ClientConfig clientConfig = ClientConfig.create() + .setEndpoint("https://your-company.absmartly.io/v1") + .setAPIKey("YOUR-API-KEY") + .setApplication("website") + .setEnvironment("development"); + +final Client client = Client.create(clientConfig, httpClient); + +final ABsmartlyConfig sdkConfig = ABsmartlyConfig.create() + .setClient(client); + +final ABsmartly sdk = ABsmartly.create(sdkConfig); + +final ContextConfig contextConfig = ContextConfig.create() + .setUnit("session_id", "abc123"); + +final Context context = sdk.createContext(contextConfig) + .waitUntilReady(); ``` -#### Overriding treatment variants -During development, for example, it is useful to force a treatment for an experiment. This can be achieved with the `override()` and/or `overrides()` methods. -The `setOverride()` and `setOverrides()` methods can be called before the context is ready. +### Request Cancellation with CompletableFuture + +Cancel inflight requests when user navigates away: + ```java - context.setOverride("exp_test_experiment", 1); // force variant 1 of treatment - context.setOverrides(Map.of( - "exp_test_experiment", 1, - "exp_another_experiment", 0 - )); +import com.absmartly.sdk.*; +import java8.util.concurrent.CompletableFuture; +import java.util.concurrent.*; + +public class CancellableContextExample { + + public static void main(String[] args) throws Exception { + final ABsmartly sdk = ABsmartly.builder() + .endpoint("https://your-company.absmartly.io/v1") + .apiKey("YOUR-API-KEY") + .application("website") + .environment("development") + .build(); + + final ContextConfig contextConfig = ContextConfig.create() + .setUnit("session_id", "abc123"); + + final Context context = sdk.createContext(contextConfig); + + // Create future for context initialization + final CompletableFuture future = context.waitUntilReadyAsync(); + + // Cancel after 1.5 seconds if not ready + final ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(1); + scheduler.schedule(() -> { + if (!future.isDone()) { + future.cancel(true); + System.out.println("Context creation cancelled"); + } + }, 1500, TimeUnit.MILLISECONDS); + + try { + final Context readyContext = future.get(); + System.out.println("Context ready!"); + readyContext.close(); + } catch (CancellationException e) { + System.out.println("Context creation was cancelled"); + } catch (ExecutionException e) { + System.out.println("Context creation failed: " + e.getCause()); + } finally { + scheduler.shutdown(); + } + } +} +``` + +### Android Activity Lifecycle Cancellation + +```java +import android.os.Bundle; +import androidx.appcompat.app.AppCompatActivity; +import com.absmartly.sdk.*; +import java8.util.concurrent.CompletableFuture; + +public class MainActivity extends AppCompatActivity { + + private static ABsmartly sdk; // Initialize SDK once (typically in Application class) + private CompletableFuture contextFuture; + private Context absmartlyContext; + + @Override + protected void onCreate(Bundle savedInstanceState) { + super.onCreate(savedInstanceState); + + final ContextConfig contextConfig = ContextConfig.create() + .setUnit("device_id", getDeviceId()); + + absmartlyContext = sdk.createContext(contextConfig); + contextFuture = absmartlyContext.waitUntilReadyAsync(); + + contextFuture.thenAccept(ctx -> { + runOnUiThread(() -> setupUI(ctx)); + }); + } + + @Override + protected void onDestroy() { + super.onDestroy(); + + // Cancel ongoing context creation if activity is destroyed + if (contextFuture != null && !contextFuture.isDone()) { + contextFuture.cancel(true); + } + + if (absmartlyContext != null) { + absmartlyContext.close(); + } + } +} ``` ## About A/B Smartly + **A/B Smartly** is the leading provider of state-of-the-art, on-premises, full-stack experimentation platforms for engineering and product teams that want to confidently deploy features as fast as they can develop them. A/B Smartly's real-time analytics helps engineering and product teams ensure that new features will improve the customer experience without breaking or degrading performance and/or business metrics. ### Have a look at our growing list of clients and SDKs: -- [Java SDK](https://www.github.com/absmartly/java-sdk) +- [Java SDK](https://www.github.com/absmartly/java-sdk) (this package) - [JavaScript SDK](https://www.github.com/absmartly/javascript-sdk) - [PHP SDK](https://www.github.com/absmartly/php-sdk) - [Swift SDK](https://www.github.com/absmartly/swift-sdk) - [Vue2 SDK](https://www.github.com/absmartly/vue2-sdk) +- [Vue3 SDK](https://www.github.com/absmartly/vue3-sdk) +- [React SDK](https://www.github.com/absmartly/react-sdk) +- [Python3 SDK](https://www.github.com/absmartly/python3-sdk) +- [Go SDK](https://www.github.com/absmartly/go-sdk) +- [Ruby SDK](https://www.github.com/absmartly/ruby-sdk) +- [.NET SDK](https://www.github.com/absmartly/dotnet-sdk) +- [Dart SDK](https://www.github.com/absmartly/dart-sdk) +- [Flutter SDK](https://www.github.com/absmartly/flutter-sdk) diff --git a/build.gradle b/build.gradle index 6af3564..3dcb30e 100644 --- a/build.gradle +++ b/build.gradle @@ -2,13 +2,11 @@ plugins { id "java" id "groovy" id "jacoco" - id "findbugs" - id "com.diffplug.spotless" version "5.8.2" - id "com.adarshr.test-logger" version "2.1.1" - id "org.barfuin.gradle.jacocolog" version "1.2.3" - id "io.github.gradle-nexus.publish-plugin" version "1.1.0" - id "org.owasp.dependencycheck" version "7.3.0" + id "com.diffplug.spotless" version "6.25.0" + id "com.adarshr.test-logger" version "4.0.0" + id "org.barfuin.gradle.jacocolog" version "3.1.0" + id "io.github.gradle-nexus.publish-plugin" version "1.3.0" } @@ -21,8 +19,8 @@ ext { jacksonVersion = "2.13.4.2" jacksonDataTypeVersion = "2.13.4" - junitVersion = "5.7.0" - mockitoVersion = "3.6.28" + junitVersion = "5.10.2" + mockitoVersion = "5.11.0" } @@ -30,17 +28,15 @@ allprojects { group = GROUP_ID apply plugin: "java" - apply plugin: "org.owasp.dependencycheck" apply from: rootProject.file("gradle/repositories.gradle") apply from: rootProject.file("gradle/spotless.gradle") - apply from: rootProject.file("gradle/findbugs.gradle") apply from: rootProject.file("gradle/test-logger.gradle") apply from: rootProject.file("gradle/jacoco.gradle") apply from: rootProject.file("gradle/coverage-logger.gradle") compileJava { - sourceCompatibility = "1.6" - targetCompatibility = "1.6" + sourceCompatibility = "1.8" + targetCompatibility = "1.8" } } diff --git a/core-api/build.gradle b/core-api/build.gradle index ca14315..c2ca4cd 100644 --- a/core-api/build.gradle +++ b/core-api/build.gradle @@ -26,18 +26,21 @@ dependencies { testImplementation group: "org.junit.jupiter", name: "junit-jupiter-params", version: junitVersion testRuntimeOnly group: "org.junit.jupiter", name: "junit-jupiter-engine", version: junitVersion testImplementation group: "org.mockito", name: "mockito-core", version: mockitoVersion - testImplementation group: "org.mockito", name: "mockito-inline", version: mockitoVersion testImplementation group: "org.mockito", name: "mockito-junit-jupiter", version: mockitoVersion } check.dependsOn jacocoTestCoverageVerification +def jacocoExcludes = [ + "com/absmartly/sdk/json/**/*", + "com/absmartly/sdk/deprecated/**/*", + "com/absmartly/sdk/java/**/*", +] + jacocoTestReport { afterEvaluate { getClassDirectories().setFrom(classDirectories.files.collect { - fileTree(dir: it, exclude: [ - "com/absmartly/core-api/json/**/*" - ]) + fileTree(dir: it, exclude: jacocoExcludes) }) } } @@ -67,9 +70,7 @@ jacocoTestCoverageVerification { afterEvaluate { getClassDirectories().setFrom(classDirectories.files.collect { - fileTree(dir: it, exclude: [ - "com/absmartly/core-api/json/**/*" - ]) + fileTree(dir: it, exclude: jacocoExcludes) }) } } @@ -77,6 +78,10 @@ jacocoTestCoverageVerification { test { useJUnitPlatform() + jvmArgs '--add-opens', 'java.base/java.lang=ALL-UNNAMED', + '--add-opens', 'java.base/java.lang.reflect=ALL-UNNAMED', + '--add-opens', 'java.base/java.util=ALL-UNNAMED', + '--add-opens', 'java.base/java.util.concurrent=ALL-UNNAMED' } publishToSonatype.dependsOn check diff --git a/core-api/src/main/java/com/absmartly/sdk/ABSmartly.java b/core-api/src/main/java/com/absmartly/sdk/ABSmartly.java deleted file mode 100644 index 5f70c63..0000000 --- a/core-api/src/main/java/com/absmartly/sdk/ABSmartly.java +++ /dev/null @@ -1,95 +0,0 @@ -package com.absmartly.sdk; - -import java.io.Closeable; -import java.io.IOException; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ScheduledThreadPoolExecutor; -import java.util.concurrent.TimeUnit; -import java8.util.concurrent.CompletableFuture; - -import javax.annotation.Nonnull; - -import com.absmartly.sdk.java.time.Clock; -import com.absmartly.sdk.json.ContextData; - -public class ABSmartly implements Closeable { - public static ABSmartly create(@Nonnull ABSmartlyConfig config) { - return new ABSmartly(config); - } - - private ABSmartly(@Nonnull ABSmartlyConfig config) { - contextDataProvider_ = config.getContextDataProvider(); - contextEventHandler_ = config.getContextEventHandler(); - contextEventLogger_ = config.getContextEventLogger(); - variableParser_ = config.getVariableParser(); - audienceDeserializer_ = config.getAudienceDeserializer(); - scheduler_ = config.getScheduler(); - - if ((contextDataProvider_ == null) || (contextEventHandler_ == null)) { - client_ = config.getClient(); - if (client_ == null) { - throw new IllegalArgumentException("Missing Client instance"); - } - - if (contextDataProvider_ == null) { - contextDataProvider_ = new DefaultContextDataProvider(client_); - } - - if (contextEventHandler_ == null) { - contextEventHandler_ = new DefaultContextEventHandler(client_); - } - } - - if (variableParser_ == null) { - variableParser_ = new DefaultVariableParser(); - } - - if (audienceDeserializer_ == null) { - audienceDeserializer_ = new DefaultAudienceDeserializer(); - } - - if (scheduler_ == null) { - scheduler_ = new ScheduledThreadPoolExecutor(1); - } - } - - public Context createContext(@Nonnull ContextConfig config) { - return Context.create(Clock.systemUTC(), config, scheduler_, contextDataProvider_.getContextData(), - contextDataProvider_, contextEventHandler_, contextEventLogger_, variableParser_, - new AudienceMatcher(audienceDeserializer_)); - } - - public Context createContextWith(@Nonnull ContextConfig config, ContextData data) { - return Context.create(Clock.systemUTC(), config, scheduler_, CompletableFuture.completedFuture(data), - contextDataProvider_, contextEventHandler_, contextEventLogger_, variableParser_, - new AudienceMatcher(audienceDeserializer_)); - } - - public CompletableFuture getContextData() { - return contextDataProvider_.getContextData(); - } - - @Override - public void close() throws IOException { - if (client_ != null) { - client_.close(); - client_ = null; - } - - if (scheduler_ != null) { - try { - scheduler_.awaitTermination(5000, TimeUnit.MILLISECONDS); - } catch (InterruptedException ignored) {} - scheduler_ = null; - } - } - - private Client client_; - private ContextDataProvider contextDataProvider_; - private ContextEventHandler contextEventHandler_; - private ContextEventLogger contextEventLogger_; - private VariableParser variableParser_; - - private AudienceDeserializer audienceDeserializer_; - private ScheduledExecutorService scheduler_; -} diff --git a/core-api/src/main/java/com/absmartly/sdk/ABsmartly.java b/core-api/src/main/java/com/absmartly/sdk/ABsmartly.java new file mode 100644 index 0000000..bcaac44 --- /dev/null +++ b/core-api/src/main/java/com/absmartly/sdk/ABsmartly.java @@ -0,0 +1,177 @@ +package com.absmartly.sdk; + +import java.io.Closeable; +import java.io.IOException; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledThreadPoolExecutor; +import java.util.concurrent.TimeUnit; +import java8.util.concurrent.CompletableFuture; + +import javax.annotation.Nonnull; + +import com.absmartly.sdk.java.time.Clock; +import com.absmartly.sdk.json.ContextData; + +public class ABsmartly implements Closeable { + public static ABsmartly create(@Nonnull ABsmartlyConfig config) { + return new ABsmartly(config); + } + + public static Builder builder() { + return new Builder(); + } + + public static class Builder { + private String endpoint; + private String apiKey; + private String application; + private String environment; + private ContextEventLogger eventLogger; + + Builder() {} + + public Builder endpoint(@Nonnull String endpoint) { + this.endpoint = endpoint; + return this; + } + + public Builder apiKey(@Nonnull String apiKey) { + this.apiKey = apiKey; + return this; + } + + public Builder application(@Nonnull String application) { + this.application = application; + return this; + } + + public Builder environment(@Nonnull String environment) { + this.environment = environment; + return this; + } + + public Builder eventLogger(@Nonnull ContextEventLogger eventLogger) { + this.eventLogger = eventLogger; + return this; + } + + public ABsmartly build() { + if (endpoint == null) throw new IllegalArgumentException("endpoint is required"); + if (apiKey == null) throw new IllegalArgumentException("apiKey is required"); + if (application == null) throw new IllegalArgumentException("application is required"); + if (environment == null) throw new IllegalArgumentException("environment is required"); + + final ClientConfig clientConfig = ClientConfig.create() + .setEndpoint(endpoint) + .setAPIKey(apiKey) + .setApplication(application) + .setEnvironment(environment); + + final ABsmartlyConfig config = ABsmartlyConfig.create() + .setClient(Client.create(clientConfig)); + + if (eventLogger != null) { + config.setContextEventLogger(eventLogger); + } + + return create(config); + } + } + + protected ABsmartly(@Nonnull ABsmartlyConfig config) { + contextDataProvider_ = config.getContextDataProvider(); + contextEventHandler_ = config.getContextPublisher(); + contextEventLogger_ = config.getContextEventLogger(); + variableParser_ = config.getVariableParser(); + audienceDeserializer_ = config.getAudienceDeserializer(); + scheduler_ = config.getScheduler(); + + if ((contextDataProvider_ == null) || (contextEventHandler_ == null)) { + client_ = config.getClient(); + if (client_ == null) { + throw new IllegalArgumentException("Missing Client instance"); + } + + if (contextDataProvider_ == null) { + contextDataProvider_ = new DefaultContextDataProvider(client_); + } + + if (contextEventHandler_ == null) { + contextEventHandler_ = new DefaultContextPublisher(client_); + } + } + + if (variableParser_ == null) { + variableParser_ = new DefaultVariableParser(); + } + + if (audienceDeserializer_ == null) { + audienceDeserializer_ = new DefaultAudienceDeserializer(); + } + + if (scheduler_ == null) { + scheduler_ = new ScheduledThreadPoolExecutor(1); + } + } + + public Context createContext(@Nonnull ContextConfig config) { + checkNotClosed(); + return Context.create(Clock.systemUTC(), config, scheduler_, contextDataProvider_.getContextData(), + contextDataProvider_, contextEventHandler_, contextEventLogger_, variableParser_, + new AudienceMatcher(audienceDeserializer_)); + } + + public Context createContextWith(@Nonnull ContextConfig config, ContextData data) { + checkNotClosed(); + return Context.create(Clock.systemUTC(), config, scheduler_, CompletableFuture.completedFuture(data), + contextDataProvider_, contextEventHandler_, contextEventLogger_, variableParser_, + new AudienceMatcher(audienceDeserializer_)); + } + + public CompletableFuture getContextData() { + checkNotClosed(); + return contextDataProvider_.getContextData(); + } + + private void checkNotClosed() { + if (closed_) { + throw new IllegalStateException("ABsmartly instance is closed"); + } + } + + @Override + public void close() throws IOException { + if (closed_) { + return; + } + closed_ = true; + + try { + if (client_ != null) { + client_.close(); + } + } finally { + if (scheduler_ != null) { + scheduler_.shutdown(); + try { + if (!scheduler_.awaitTermination(5000, TimeUnit.MILLISECONDS)) { + scheduler_.shutdownNow(); + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + scheduler_.shutdownNow(); + } + } + } + } + + private volatile boolean closed_; + private Client client_; + private ContextDataProvider contextDataProvider_; + private ContextPublisher contextEventHandler_; + private ContextEventLogger contextEventLogger_; + private VariableParser variableParser_; + + private AudienceDeserializer audienceDeserializer_; + private ScheduledExecutorService scheduler_; +} diff --git a/core-api/src/main/java/com/absmartly/sdk/ABSmartlyConfig.java b/core-api/src/main/java/com/absmartly/sdk/ABsmartlyConfig.java similarity index 57% rename from core-api/src/main/java/com/absmartly/sdk/ABSmartlyConfig.java rename to core-api/src/main/java/com/absmartly/sdk/ABsmartlyConfig.java index c4346d0..d7ad9d5 100644 --- a/core-api/src/main/java/com/absmartly/sdk/ABSmartlyConfig.java +++ b/core-api/src/main/java/com/absmartly/sdk/ABsmartlyConfig.java @@ -4,27 +4,47 @@ import javax.annotation.Nonnull; -public class ABSmartlyConfig { - public static ABSmartlyConfig create() { - return new ABSmartlyConfig(); +public class ABsmartlyConfig { + public static ABsmartlyConfig create() { + return new ABsmartlyConfig(); } - private ABSmartlyConfig() {} + protected ABsmartlyConfig() {} public ContextDataProvider getContextDataProvider() { return contextDataProvider_; } - public ABSmartlyConfig setContextDataProvider(@Nonnull final ContextDataProvider contextDataProvider) { + public ABsmartlyConfig setContextDataProvider(@Nonnull final ContextDataProvider contextDataProvider) { contextDataProvider_ = contextDataProvider; return this; } - public ContextEventHandler getContextEventHandler() { + public ContextPublisher getContextPublisher() { return contextEventHandler_; } - public ABSmartlyConfig setContextEventHandler(@Nonnull final ContextEventHandler contextEventHandler) { + public ABsmartlyConfig setContextPublisher(@Nonnull final ContextPublisher contextPublisher) { + contextEventHandler_ = contextPublisher; + return this; + } + + /** + * @deprecated Use {@link #getContextPublisher()} instead. + */ + @Deprecated + public ContextEventHandler getContextEventHandler() { + if (contextEventHandler_ instanceof ContextEventHandler) { + return (ContextEventHandler) contextEventHandler_; + } + return null; + } + + /** + * @deprecated Use {@link #setContextPublisher(ContextPublisher)} instead. + */ + @Deprecated + public ABsmartlyConfig setContextEventHandler(@Nonnull final ContextEventHandler contextEventHandler) { contextEventHandler_ = contextEventHandler; return this; } @@ -33,7 +53,7 @@ public VariableParser getVariableParser() { return variableParser_; } - public ABSmartlyConfig setVariableParser(@Nonnull final VariableParser variableParser) { + public ABsmartlyConfig setVariableParser(@Nonnull final VariableParser variableParser) { variableParser_ = variableParser; return this; } @@ -42,7 +62,7 @@ public ScheduledExecutorService getScheduler() { return scheduler_; } - public ABSmartlyConfig setScheduler(@Nonnull final ScheduledExecutorService scheduler) { + public ABsmartlyConfig setScheduler(@Nonnull final ScheduledExecutorService scheduler) { scheduler_ = scheduler; return this; } @@ -51,7 +71,7 @@ public ContextEventLogger getContextEventLogger() { return contextEventLogger_; } - public ABSmartlyConfig setContextEventLogger(@Nonnull final ContextEventLogger logger) { + public ABsmartlyConfig setContextEventLogger(@Nonnull final ContextEventLogger logger) { contextEventLogger_ = logger; return this; } @@ -60,7 +80,7 @@ public AudienceDeserializer getAudienceDeserializer() { return audienceDeserializer_; } - public ABSmartlyConfig setAudienceDeserializer(@Nonnull final AudienceDeserializer audienceDeserializer) { + public ABsmartlyConfig setAudienceDeserializer(@Nonnull final AudienceDeserializer audienceDeserializer) { audienceDeserializer_ = audienceDeserializer; return this; } @@ -69,13 +89,13 @@ public Client getClient() { return client_; } - public ABSmartlyConfig setClient(Client client) { + public ABsmartlyConfig setClient(Client client) { client_ = client; return this; } private ContextDataProvider contextDataProvider_; - private ContextEventHandler contextEventHandler_; + private ContextPublisher contextEventHandler_; private ContextEventLogger contextEventLogger_; private VariableParser variableParser_; diff --git a/core-api/src/main/java/com/absmartly/sdk/AudienceMatcher.java b/core-api/src/main/java/com/absmartly/sdk/AudienceMatcher.java index f3b9a7a..3fb3aea 100644 --- a/core-api/src/main/java/com/absmartly/sdk/AudienceMatcher.java +++ b/core-api/src/main/java/com/absmartly/sdk/AudienceMatcher.java @@ -25,6 +25,9 @@ public boolean get() { } public Result evaluate(String audience, Map attributes) { + if (audience == null || audience.isEmpty()) { + return null; + } final byte[] bytes = audience.getBytes(StandardCharsets.UTF_8); final Map audienceMap = deserializer_.deserialize(bytes, 0, bytes.length); if (audienceMap != null) { diff --git a/core-api/src/main/java/com/absmartly/sdk/Client.java b/core-api/src/main/java/com/absmartly/sdk/Client.java index e172461..109c532 100644 --- a/core-api/src/main/java/com/absmartly/sdk/Client.java +++ b/core-api/src/main/java/com/absmartly/sdk/Client.java @@ -13,10 +13,14 @@ import javax.annotation.Nonnull; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import com.absmartly.sdk.json.ContextData; import com.absmartly.sdk.json.PublishEvent; public class Client implements Closeable { + private static final Logger log = LoggerFactory.getLogger(Client.class); static public Client create(@Nonnull final ClientConfig config) { return new Client(config, DefaultHTTPClient.create(DefaultHTTPClientConfig.create())); } @@ -31,6 +35,15 @@ static public Client create(@Nonnull final ClientConfig config, @Nonnull final H throw new IllegalArgumentException("Missing Endpoint configuration"); } + if (!endpoint.startsWith("https://")) { + if (endpoint.startsWith("http://")) { + log.warn("ABsmartly SDK endpoint is not using HTTPS. API keys will be transmitted in plaintext: {}", + endpoint); + } else { + throw new IllegalArgumentException("Endpoint must use http:// or https:// protocol: " + endpoint); + } + } + final String apiKey = config.getAPIKey(); if ((apiKey == null) || apiKey.isEmpty()) { throw new IllegalArgumentException("Missing APIKey configuration"); @@ -46,7 +59,8 @@ static public Client create(@Nonnull final ClientConfig config, @Nonnull final H throw new IllegalArgumentException("Missing Environment configuration"); } - url_ = endpoint + "/context"; + final String normalizedEndpoint = endpoint.endsWith("/") ? endpoint.substring(0, endpoint.length() - 1) : endpoint; + url_ = normalizedEndpoint + "/context"; httpClient_ = httpClient; deserializer_ = config.getContextDataDeserializer(); serializer_ = config.getContextEventSerializer(); @@ -86,8 +100,18 @@ public void accept(HTTPClient.Response response) { final int code = response.getStatusCode(); if ((code / 100) == 2) { final byte[] content = response.getContent(); - dataFuture.complete( - deserializer_.deserialize(response.getContent(), 0, content.length)); + if (content == null || content.length == 0) { + dataFuture.completeExceptionally(new IllegalStateException( + "Empty response body from context data endpoint")); + } else { + final ContextData result = deserializer_.deserialize(content, 0, content.length); + if (result != null) { + dataFuture.complete(result); + } else { + dataFuture.completeExceptionally(new IllegalStateException( + "Failed to deserialize context data response")); + } + } } else { dataFuture.completeExceptionally(new Exception(response.getStatusMessage())); } diff --git a/core-api/src/main/java/com/absmartly/sdk/Context.java b/core-api/src/main/java/com/absmartly/sdk/Context.java index a3d99b3..1c37449 100644 --- a/core-api/src/main/java/com/absmartly/sdk/Context.java +++ b/core-api/src/main/java/com/absmartly/sdk/Context.java @@ -7,6 +7,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.locks.ReentrantLock; import java.util.concurrent.locks.ReentrantReadWriteLock; import java8.util.concurrent.CompletableFuture; @@ -17,6 +18,9 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import com.absmartly.sdk.internal.Algorithm; import com.absmartly.sdk.internal.Concurrency; import com.absmartly.sdk.internal.VariantAssigner; @@ -26,10 +30,12 @@ import com.absmartly.sdk.json.*; public class Context implements Closeable { + private static final Logger log = LoggerFactory.getLogger(Context.class); + public static Context create(@Nonnull final Clock clock, @Nonnull final ContextConfig config, @Nonnull final ScheduledExecutorService scheduler, @Nonnull final CompletableFuture dataFuture, @Nonnull final ContextDataProvider dataProvider, - @Nonnull final ContextEventHandler eventHandler, @Nullable final ContextEventLogger eventLogger, + @Nonnull final ContextPublisher eventHandler, @Nullable final ContextEventLogger eventLogger, @Nonnull final VariableParser variableParser, @Nonnull AudienceMatcher audienceMatcher) { return new Context(clock, config, scheduler, dataFuture, dataProvider, eventHandler, eventLogger, variableParser, audienceMatcher); @@ -37,7 +43,7 @@ public static Context create(@Nonnull final Clock clock, @Nonnull final ContextC private Context(Clock clock, ContextConfig config, ScheduledExecutorService scheduler, CompletableFuture dataFuture, ContextDataProvider dataProvider, - ContextEventHandler eventHandler, ContextEventLogger eventLogger, VariableParser variableParser, + ContextPublisher eventHandler, ContextEventLogger eventLogger, VariableParser variableParser, AudienceMatcher audienceMatcher) { clock_ = clock; publishDelay_ = config.getPublishDelay(); @@ -87,13 +93,16 @@ public Void apply(Throwable exception) { } }); } else { - readyFuture_ = new CompletableFuture(); + final CompletableFuture newReadyFuture = new CompletableFuture(); + readyFuture_.set(newReadyFuture); dataFuture.thenAccept(new Consumer() { @Override public void accept(ContextData data) { Context.this.setData(data); - readyFuture_.complete(null); - readyFuture_ = null; + final CompletableFuture rf = readyFuture_.getAndSet(COMPLETED_VOID_FUTURE); + if (rf != null) { + rf.complete(null); + } Context.this.logEvent(ContextEventLogger.EventType.Ready, data); @@ -105,8 +114,10 @@ public void accept(ContextData data) { @Override public Void apply(Throwable exception) { Context.this.setDataFailed(exception); - readyFuture_.complete(null); - readyFuture_ = null; + final CompletableFuture rf = readyFuture_.getAndSet(COMPLETED_VOID_FUTURE); + if (rf != null) { + rf.complete(null); + } Context.this.logError(exception); @@ -132,22 +143,34 @@ public boolean isClosing() { return !closed_.get() && closing_.get(); } + public boolean isFinalized() { + return isClosed(); + } + + public boolean isFinalizing() { + return isClosing(); + } + public CompletableFuture waitUntilReadyAsync() { if (data_ != null) { return CompletableFuture.completedFuture(this); } else { - return readyFuture_.thenApply(new Function() { - @Override - public Context apply(Void k) { - return Context.this; - } - }); + final CompletableFuture rf = readyFuture_.get(); + if (rf != null) { + return rf.thenApply(new Function() { + @Override + public Context apply(Void k) { + return Context.this; + } + }); + } + return CompletableFuture.completedFuture(this); } } public Context waitUntilReady() { if (data_ == null) { - final CompletableFuture future = readyFuture_; // cache here to avoid locking + final CompletableFuture future = readyFuture_.get(); // cache here to avoid locking if (future != null && !future.isDone()) { future.join(); } @@ -156,7 +179,9 @@ public Context waitUntilReady() { } public String[] getExperiments() { - checkReady(true); + if (!isReady() || isClosed() || isClosing()) { + return new String[0]; + } try { dataLock_.readLock().lock(); @@ -174,6 +199,10 @@ public String[] getExperiments() { } public String[] getCustomFieldKeys() { + if (!isReady() || isClosed() || isClosing()) { + return new String[0]; + } + final Set keys = new HashSet(); try { @@ -193,6 +222,10 @@ public String[] getCustomFieldKeys() { } public Object getCustomFieldValue(@Nonnull final String experimentName, @Nonnull final String key) { + if (!isReady() || isClosed() || isClosing()) { + return null; + } + try { dataLock_.readLock().lock(); final ContextExperiment experiment = index_.get(experimentName); @@ -209,6 +242,10 @@ public Object getCustomFieldValue(@Nonnull final String experimentName, @Nonnull } public Object getCustomFieldValueType(@Nonnull final String experimentName, @Nonnull final String key) { + if (!isReady() || isClosed() || isClosing()) { + return null; + } + try { dataLock_.readLock().lock(); final ContextExperiment experiment = index_.get(experimentName); @@ -236,8 +273,6 @@ public ContextData getData() { } public void setOverride(@Nonnull final String experimentName, final int variant) { - checkNotClosed(); - Concurrency.putRW(contextLock_, overrides_, experimentName, variant); } @@ -290,7 +325,7 @@ public void setUnit(@Nonnull final String unitType, @Nonnull final String uid) { final String previous = units_.get(unitType); if ((previous != null) && !previous.equals(uid)) { - throw new IllegalArgumentException(String.format("Unit '%s' already set.", unitType)); + throw new IllegalArgumentException(String.format("Unit '%s' UID already set.", unitType)); } final String trimmed = uid.trim(); @@ -343,6 +378,7 @@ public void setAttribute(@Nonnull final String name, @Nullable final Object valu checkNotClosed(); Concurrency.addRW(contextLock_, attributes_, new Attribute(name, value, clock_.millis())); + attrsSeq_.incrementAndGet(); } public Map getAttributes() { @@ -368,7 +404,13 @@ public void setAttributes(@Nonnull final Map attributes) { } public int getTreatment(@Nonnull final String experimentName) { - checkReady(true); + if (!isReady()) { + return 0; + } + + if (isClosed() || isClosing()) { + return 0; + } final Assignment assignment = getAssignment(experimentName); if (!assignment.exposed.get()) { @@ -408,13 +450,17 @@ private void queueExposure(final Assignment assignment) { } public int peekTreatment(@Nonnull final String experimentName) { - checkReady(true); + if (!isReady() || isClosed() || isClosing()) { + return 0; + } return getAssignment(experimentName).variant; } public Map> getVariableKeys() { - checkReady(true); + if (!isReady() || isClosed() || isClosing()) { + return new HashMap>(); + } final Map> variableKeys = new HashMap>(indexVariables_.size()); @@ -437,7 +483,9 @@ public Map> getVariableKeys() { } public Object getVariableValue(@Nonnull final String key, final Object defaultValue) { - checkReady(true); + if (!isReady() || isClosed() || isClosing()) { + return defaultValue; + } final Assignment assignment = getVariableAssignment(key); if (assignment != null) { @@ -455,7 +503,9 @@ public Object getVariableValue(@Nonnull final String key, final Object defaultVa } public Object peekVariableValue(@Nonnull final String key, final Object defaultValue) { - checkReady(true); + if (!isReady() || isClosed() || isClosing()) { + return defaultValue; + } final Assignment assignment = getVariableAssignment(key); if (assignment != null) { @@ -507,14 +557,15 @@ public CompletableFuture refreshAsync() { checkNotClosed(); if (refreshing_.compareAndSet(false, true)) { - refreshFuture_ = new CompletableFuture(); + final CompletableFuture newRefreshFuture = new CompletableFuture(); + refreshFuture_.set(newRefreshFuture); dataProvider_.getContextData().thenAccept(new Consumer() { @Override public void accept(ContextData data) { Context.this.setData(data); refreshing_.set(false); - refreshFuture_.complete(null); + newRefreshFuture.complete(null); Context.this.logEvent(ContextEventLogger.EventType.Refresh, data); } @@ -522,7 +573,7 @@ public void accept(ContextData data) { @Override public Void apply(Throwable exception) { refreshing_.set(false); - refreshFuture_.completeExceptionally(exception); + newRefreshFuture.completeExceptionally(exception); Context.this.logError(exception); return null; @@ -530,7 +581,7 @@ public Void apply(Throwable exception) { }); } - final CompletableFuture future = refreshFuture_; + final CompletableFuture future = refreshFuture_.get(); if (future != null) { return future; } @@ -548,14 +599,15 @@ public CompletableFuture closeAsync() { clearRefreshTimer(); if (pendingCount_.get() > 0) { - closingFuture_ = new CompletableFuture(); + final CompletableFuture newClosingFuture = new CompletableFuture(); + closingFuture_.set(newClosingFuture); flush().thenAccept(new Consumer() { @Override public void accept(Void x) { closed_.set(true); closing_.set(false); - closingFuture_.complete(null); + newClosingFuture.complete(null); Context.this.logEvent(ContextEventLogger.EventType.Close, null); } @@ -564,14 +616,13 @@ public void accept(Void x) { public Void apply(Throwable exception) { closed_.set(true); closing_.set(false); - closingFuture_.completeExceptionally(exception); - // event logger gets this error during publish + newClosingFuture.completeExceptionally(exception); return null; } }); - return closingFuture_; + return newClosingFuture; } else { closed_.set(true); closing_.set(false); @@ -580,7 +631,7 @@ public Void apply(Throwable exception) { } } - final CompletableFuture future = closingFuture_; + final CompletableFuture future = closingFuture_.get(); if (future != null) { return future; } @@ -594,6 +645,16 @@ public void close() { closeAsync().join(); } + @Deprecated + public CompletableFuture finalizeAsync() { + return closeAsync(); + } + + @Deprecated + public void finalize() { + close(); + } + private CompletableFuture flush() { clearTimeout(); @@ -628,21 +689,31 @@ private CompletableFuture flush() { final PublishEvent event = new PublishEvent(); event.hashed = true; event.publishedAt = clock_.millis(); - event.units = Algorithm.mapSetToArray(units_.entrySet(), new Unit[0], - new Function, Unit>() { - @Override - public Unit apply(Map.Entry entry) { - return new Unit(entry.getKey(), - new String(getUnitHash(entry.getKey(), entry.getValue()), - StandardCharsets.US_ASCII)); - } - }); - event.attributes = attributes_.isEmpty() ? null : attributes_.toArray(new Attribute[0]); + + try { + contextLock_.writeLock().lock(); + event.units = Algorithm.mapSetToArray(units_.entrySet(), new Unit[0], + new Function, Unit>() { + @Override + public Unit apply(Map.Entry entry) { + return new Unit(entry.getKey(), + new String(getUnitHash(entry.getKey(), entry.getValue()), + StandardCharsets.US_ASCII)); + } + }); + event.attributes = attributes_.isEmpty() ? null : attributes_.toArray(new Attribute[0]); + } finally { + contextLock_.writeLock().unlock(); + } event.exposures = exposures; event.goals = achievements; final CompletableFuture result = new CompletableFuture(); + final Exposure[] finalExposures = exposures; + final GoalAchievement[] finalAchievements = achievements; + final int finalEventCount = eventCount; + eventHandler_.publish(this, event).thenRunAsync(new Runnable() { @Override public void run() { @@ -652,6 +723,23 @@ public void run() { }).exceptionally(new Function() { @Override public Void apply(Throwable throwable) { + try { + eventLock_.lock(); + if (finalExposures != null) { + for (int i = finalExposures.length - 1; i >= 0; i--) { + exposures_.add(0, finalExposures[i]); + } + } + if (finalAchievements != null) { + for (int i = finalAchievements.length - 1; i >= 0; i--) { + achievements_.add(0, finalAchievements[i]); + } + } + pendingCount_.set(finalEventCount); + } finally { + eventLock_.unlock(); + } + Context.this.logError(throwable); result.completeExceptionally(throwable); @@ -678,15 +766,15 @@ public Void apply(Throwable throwable) { private void checkNotClosed() { if (closed_.get()) { - throw new IllegalStateException("ABSmartly Context is closed"); + throw new IllegalStateException("ABsmartly Context is finalized."); } else if (closing_.get()) { - throw new IllegalStateException("ABSmartly Context is closing"); + throw new IllegalStateException("ABsmartly Context is closing."); } } private void checkReady(final boolean expectNotClosed) { if (!isReady()) { - throw new IllegalStateException("ABSmartly Context is not yet ready"); + throw new IllegalStateException("ABsmartly Context is not yet ready."); } else if (expectNotClosed) { checkNotClosed(); } @@ -694,12 +782,28 @@ private void checkReady(final boolean expectNotClosed) { private boolean experimentMatches(final Experiment experiment, final Assignment assignment) { return experiment.id == assignment.id && - experiment.unitType.equals(assignment.unitType) && + (experiment.unitType != null && experiment.unitType.equals(assignment.unitType)) && experiment.iteration == assignment.iteration && experiment.fullOnVariant == assignment.fullOnVariant && Arrays.equals(experiment.trafficSplit, assignment.trafficSplit); } + private boolean audienceMatches(final Experiment experiment, final Assignment assignment) { + if (experiment.audience != null && experiment.audience.length() > 0) { + if (attrsSeq_.get() > assignment.attrsSeq) { + final Map attrs = buildAttributesMap(); + + final AudienceMatcher.Result match = audienceMatcher_.evaluate(experiment.audience, attrs); + final boolean newAudienceMismatch = (match != null) ? !match.get() : false; + + if (newAudienceMismatch != assignment.audienceMismatch) { + return false; + } + } + } + return true; + } + private static class Assignment { int id; int iteration; @@ -715,7 +819,8 @@ private static class Assignment { boolean custom; boolean audienceMismatch; - Map variables = Collections.emptyMap(); + Map variables = null; + int attrsSeq; final AtomicBoolean exposed = new AtomicBoolean(false); } @@ -743,7 +848,8 @@ private Assignment getAssignment(final String experimentName) { return assignment; } } else if ((custom == null) || custom == assignment.variant) { - if (experimentMatches(experiment.data, assignment)) { + if (experimentMatches(experiment.data, assignment) + && audienceMatches(experiment.data, assignment)) { // assignment up-to-date return assignment; } @@ -779,10 +885,7 @@ private Assignment getAssignment(final String experimentName) { final String unitType = experiment.data.unitType; if (experiment.data.audience != null && experiment.data.audience.length() > 0) { - final Map attrs = new HashMap(attributes_.size()); - for (final Attribute attr : attributes_) { - attrs.put(attr.name, attr.value); - } + final Map attrs = buildAttributesMap(); final AudienceMatcher.Result match = audienceMatcher_ .evaluate(experiment.data.audience, attrs); @@ -829,10 +932,12 @@ private Assignment getAssignment(final String experimentName) { assignment.iteration = experiment.data.iteration; assignment.trafficSplit = experiment.data.trafficSplit; assignment.fullOnVariant = experiment.data.fullOnVariant; + assignment.attrsSeq = attrsSeq_.get(); } } - if ((experiment != null) && (assignment.variant < experiment.data.variants.length)) { + if ((experiment != null) && experiment.data.variants != null && assignment.variant >= 0 + && (assignment.variant < experiment.data.variants.length)) { assignment.variables = experiment.variables.get(assignment.variant); } @@ -891,7 +996,7 @@ public VariantAssigner apply(String key) { } private void setTimeout() { - if (isReady()) { + if (isReady() && publishDelay_ >= 0) { if (timeout_ == null) { try { timeoutLock_.lock(); @@ -899,7 +1004,13 @@ private void setTimeout() { timeout_ = scheduler_.schedule(new Runnable() { @Override public void run() { - Context.this.flush(); + Context.this.flush().exceptionally(new Function() { + @Override + public Void apply(Throwable exception) { + Context.this.logError(exception); + return null; + } + }); } }, publishDelay_, TimeUnit.MILLISECONDS); } @@ -954,40 +1065,56 @@ private static class ContextCustomFieldValue { } private void setData(final ContextData data) { + if (data == null) { + throw new IllegalArgumentException("Context data cannot be null"); + } + final Map index = new HashMap(); final Map> indexVariables = new HashMap>(); for (final Experiment experiment : data.experiments) { final ContextExperiment contextExperiment = new ContextExperiment(); contextExperiment.data = experiment; - contextExperiment.variables = new ArrayList>(experiment.variants.length); + contextExperiment.variables = new ArrayList>( + experiment.variants != null ? experiment.variants.length : 0); + if (experiment.variants != null) for (final ExperimentVariant variant : experiment.variants) { if ((variant.config != null) && !variant.config.isEmpty()) { - final Map variables = variableParser_.parse(this, experiment.name, variant.name, - variant.config); - for (final String key : variables.keySet()) { - List keyExperimentVariables = indexVariables.get(key); - if (keyExperimentVariables == null) { - keyExperimentVariables = new ArrayList(); - indexVariables.put(key, keyExperimentVariables); - } + try { + final Map variables = variableParser_.parse(this, experiment.name, variant.name, + variant.config); + if (variables != null) { + for (final String key : variables.keySet()) { + List keyExperimentVariables = indexVariables.get(key); + if (keyExperimentVariables == null) { + keyExperimentVariables = new ArrayList(); + indexVariables.put(key, keyExperimentVariables); + } - int at = Collections.binarySearch(keyExperimentVariables, contextExperiment, - new Comparator() { - @Override - public int compare(ContextExperiment a, ContextExperiment b) { - return Integer.valueOf(a.data.id).compareTo(b.data.id); - } - }); + int at = Collections.binarySearch(keyExperimentVariables, contextExperiment, + new Comparator() { + @Override + public int compare(ContextExperiment a, ContextExperiment b) { + return Integer.valueOf(a.data.id).compareTo(b.data.id); + } + }); + + if (at < 0) { + at = -at - 1; + keyExperimentVariables.add(at, contextExperiment); + } + } - if (at < 0) { - at = -at - 1; - keyExperimentVariables.add(at, contextExperiment); + contextExperiment.variables.add(variables); + } else { + contextExperiment.variables.add(Collections. emptyMap()); } + } catch (Exception e) { + log.error("Failed to parse variant config for experiment '{}', variant '{}': {}", + experiment.name, variant.name, e.getMessage()); + contextExperiment.variables.add(Collections. emptyMap()); } - - contextExperiment.variables.add(variables); } else { contextExperiment.variables.add(Collections. emptyMap()); } @@ -1001,13 +1128,24 @@ public int compare(ContextExperiment a, ContextExperiment b) { value.type = customFieldValue.getType(); if (customFieldValue.getValue() != null) { - if (customFieldValue.getType().startsWith("json")) { - value.value = variableParser_.parse(this, experiment.name, customFieldValue.getValue()); - } else if (customFieldValue.getType().equals("boolean")) { - value.value = Boolean.parseBoolean(customFieldValue.getValue()); - } else if (customFieldValue.getType().equals("number")) { - value.value = Double.parseDouble(customFieldValue.getValue()); - } else { + try { + final String type = customFieldValue.getType(); + if (type != null && type.startsWith("json")) { + value.value = variableParser_.parse(this, experiment.name, customFieldValue.getValue()); + } else if (type != null && type.equals("boolean")) { + value.value = Boolean.parseBoolean(customFieldValue.getValue()); + } else if (type != null && type.equals("number")) { + value.value = Double.parseDouble(customFieldValue.getValue()); + } else { + value.value = customFieldValue.getValue(); + } + } catch (NumberFormatException e) { + log.warn("Failed to parse custom field number value for experiment '{}': {}", + experiment.name, e.getMessage()); + value.value = customFieldValue.getValue(); + } catch (Exception e) { + log.warn("Failed to parse custom field value for experiment '{}': {}", experiment.name, + e.getMessage()); value.value = customFieldValue.getValue(); } } @@ -1030,6 +1168,10 @@ public int compare(ContextExperiment a, ContextExperiment b) { } } + public Throwable readyError() { + return readyError_; + } + private void setDataFailed(final Throwable exception) { try { dataLock_.writeLock().lock(); @@ -1037,6 +1179,7 @@ private void setDataFailed(final Throwable exception) { indexVariables_ = new HashMap>(); data_ = new ContextData(); failed_ = true; + readyError_ = exception; } finally { dataLock_.writeLock().unlock(); } @@ -1057,20 +1200,29 @@ private void logError(Throwable error) { } } + private Map buildAttributesMap() { + final Map attrs = new HashMap(attributes_.size()); + for (final Attribute attr : attributes_) { + attrs.put(attr.name, attr.value); + } + return attrs; + } + private final Clock clock_; private final long publishDelay_; private final long refreshInterval_; - private final ContextEventHandler eventHandler_; + private final ContextPublisher eventHandler_; private final ContextEventLogger eventLogger_; private final ContextDataProvider dataProvider_; private final VariableParser variableParser_; private final AudienceMatcher audienceMatcher_; private final ScheduledExecutorService scheduler_; private final Map units_; - private boolean failed_; + private volatile boolean failed_; + private volatile Throwable readyError_; private final ReentrantReadWriteLock dataLock_ = new ReentrantReadWriteLock(); - private ContextData data_; + private volatile ContextData data_; private Map index_; private Map> indexVariables_; private final ReentrantReadWriteLock contextLock_ = new ReentrantReadWriteLock(); @@ -1086,15 +1238,17 @@ private void logError(Throwable error) { private final List attributes_ = new ArrayList(); private final Map overrides_; private final Map cassignments_; + private final AtomicInteger attrsSeq_ = new AtomicInteger(0); private final AtomicInteger pendingCount_ = new AtomicInteger(0); private final AtomicBoolean closing_ = new AtomicBoolean(false); private final AtomicBoolean closed_ = new AtomicBoolean(false); private final AtomicBoolean refreshing_ = new AtomicBoolean(false); - private volatile CompletableFuture readyFuture_; - private volatile CompletableFuture closingFuture_; - private volatile CompletableFuture refreshFuture_; + private static final CompletableFuture COMPLETED_VOID_FUTURE = CompletableFuture.completedFuture(null); + private final AtomicReference> readyFuture_ = new AtomicReference>(); + private final AtomicReference> closingFuture_ = new AtomicReference>(); + private final AtomicReference> refreshFuture_ = new AtomicReference>(); private final ReentrantLock timeoutLock_ = new ReentrantLock(); private volatile ScheduledFuture timeout_ = null; diff --git a/core-api/src/main/java/com/absmartly/sdk/ContextConfig.java b/core-api/src/main/java/com/absmartly/sdk/ContextConfig.java index 9bdd6ab..a6ecbfc 100644 --- a/core-api/src/main/java/com/absmartly/sdk/ContextConfig.java +++ b/core-api/src/main/java/com/absmartly/sdk/ContextConfig.java @@ -29,7 +29,7 @@ public ContextConfig setUnits(@Nonnull final Map units) { } public String getUnit(@Nonnull final String unitType) { - return units_.get(unitType); + return units_ != null ? units_.get(unitType) : null; } public Map getUnits() { @@ -53,7 +53,7 @@ public ContextConfig setAttributes(@Nonnull final Map attributes } public Object getAttribute(@Nonnull final String name) { - return this.attributes_.get(name); + return this.attributes_ != null ? this.attributes_.get(name) : null; } public Map getAttributes() { @@ -77,7 +77,7 @@ public ContextConfig setOverrides(@Nonnull final Map overrides) } public Object getOverride(@Nonnull final String experimentName) { - return this.overrides_.get(experimentName); + return this.overrides_ != null ? this.overrides_.get(experimentName) : null; } public Map getOverrides() { @@ -85,27 +85,27 @@ public Map getOverrides() { } public ContextConfig setCustomAssignment(@Nonnull final String experimentName, int variant) { - if (cassigmnents_ == null) { - cassigmnents_ = new HashMap(); + if (cassignments_ == null) { + cassignments_ = new HashMap(); } - cassigmnents_.put(experimentName, variant); + cassignments_.put(experimentName, variant); return this; } public ContextConfig setCustomAssignments(@Nonnull final Map customAssignments) { - if (cassigmnents_ == null) { - cassigmnents_ = new HashMap(customAssignments.size()); + if (cassignments_ == null) { + cassignments_ = new HashMap(customAssignments.size()); } - cassigmnents_.putAll(customAssignments); + cassignments_.putAll(customAssignments); return this; } public Object getCustomAssignment(@Nonnull final String experimentName) { - return this.cassigmnents_.get(experimentName); + return this.cassignments_ != null ? this.cassignments_.get(experimentName) : null; } public Map getCustomAssignments() { - return this.cassigmnents_; + return this.cassignments_; } public ContextEventLogger getEventLogger() { @@ -138,7 +138,7 @@ public long getRefreshInterval() { private Map units_; private Map attributes_; private Map overrides_; - private Map cassigmnents_; + private Map cassignments_; private ContextEventLogger eventLogger_; diff --git a/core-api/src/main/java/com/absmartly/sdk/ContextEventHandler.java b/core-api/src/main/java/com/absmartly/sdk/ContextEventHandler.java index c5576b3..6415367 100644 --- a/core-api/src/main/java/com/absmartly/sdk/ContextEventHandler.java +++ b/core-api/src/main/java/com/absmartly/sdk/ContextEventHandler.java @@ -1,11 +1,8 @@ package com.absmartly.sdk; -import java8.util.concurrent.CompletableFuture; - -import javax.annotation.Nonnull; - -import com.absmartly.sdk.json.PublishEvent; - -public interface ContextEventHandler { - CompletableFuture publish(@Nonnull final Context context, @Nonnull final PublishEvent event); +/** + * @deprecated Use {@link ContextPublisher} instead. + */ +@Deprecated +public interface ContextEventHandler extends ContextPublisher { } diff --git a/core-api/src/main/java/com/absmartly/sdk/ContextPublisher.java b/core-api/src/main/java/com/absmartly/sdk/ContextPublisher.java new file mode 100644 index 0000000..9b78f61 --- /dev/null +++ b/core-api/src/main/java/com/absmartly/sdk/ContextPublisher.java @@ -0,0 +1,11 @@ +package com.absmartly.sdk; + +import java8.util.concurrent.CompletableFuture; + +import javax.annotation.Nonnull; + +import com.absmartly.sdk.json.PublishEvent; + +public interface ContextPublisher { + CompletableFuture publish(@Nonnull final Context context, @Nonnull final PublishEvent event); +} diff --git a/core-api/src/main/java/com/absmartly/sdk/DefaultAudienceDeserializer.java b/core-api/src/main/java/com/absmartly/sdk/DefaultAudienceDeserializer.java index a93c0b6..465f6cb 100644 --- a/core-api/src/main/java/com/absmartly/sdk/DefaultAudienceDeserializer.java +++ b/core-api/src/main/java/com/absmartly/sdk/DefaultAudienceDeserializer.java @@ -8,19 +8,17 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.fasterxml.jackson.databind.MapperFeature; -import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.ObjectReader; +import com.absmartly.sdk.internal.JsonMapperUtils; + public class DefaultAudienceDeserializer implements AudienceDeserializer { private static final Logger log = LoggerFactory.getLogger(DefaultAudienceDeserializer.class); private final ObjectReader reader_; public DefaultAudienceDeserializer() { - final ObjectMapper objectMapper = new ObjectMapper(); - objectMapper.enable(MapperFeature.USE_STATIC_TYPING); - this.reader_ = objectMapper.readerForMapOf(Object.class); + this.reader_ = JsonMapperUtils.createStandardObjectMapper().readerForMapOf(Object.class); } @Override @@ -28,7 +26,7 @@ public Map deserialize(@Nonnull byte[] bytes, int offset, int le try { return reader_.readValue(bytes, offset, length); } catch (IOException e) { - log.error("", e); + log.error("Failed to deserialize audience data: {}", e.getMessage(), e); return null; } } diff --git a/core-api/src/main/java/com/absmartly/sdk/DefaultContextDataDeserializer.java b/core-api/src/main/java/com/absmartly/sdk/DefaultContextDataDeserializer.java index b28eb79..0d49e49 100644 --- a/core-api/src/main/java/com/absmartly/sdk/DefaultContextDataDeserializer.java +++ b/core-api/src/main/java/com/absmartly/sdk/DefaultContextDataDeserializer.java @@ -7,26 +7,23 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.fasterxml.jackson.databind.MapperFeature; -import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.ObjectReader; +import com.absmartly.sdk.internal.JsonMapperUtils; import com.absmartly.sdk.json.ContextData; public class DefaultContextDataDeserializer implements ContextDataDeserializer { private static final Logger log = LoggerFactory.getLogger(DefaultContextDataDeserializer.class); public DefaultContextDataDeserializer() { - final ObjectMapper objectMapper = new ObjectMapper(); - objectMapper.enable(MapperFeature.USE_STATIC_TYPING); - this.reader_ = objectMapper.readerFor(ContextData.class); + this.reader_ = JsonMapperUtils.createStandardObjectMapper().readerFor(ContextData.class); } public ContextData deserialize(@Nonnull final byte[] bytes, final int offset, final int length) { try { return reader_.readValue(bytes, offset, length); } catch (IOException e) { - log.error("", e); + log.error("Failed to deserialize context data: {}", e.getMessage(), e); return null; } } diff --git a/core-api/src/main/java/com/absmartly/sdk/DefaultContextEventHandler.java b/core-api/src/main/java/com/absmartly/sdk/DefaultContextEventHandler.java index 7e30584..dfb2d34 100644 --- a/core-api/src/main/java/com/absmartly/sdk/DefaultContextEventHandler.java +++ b/core-api/src/main/java/com/absmartly/sdk/DefaultContextEventHandler.java @@ -1,20 +1,13 @@ package com.absmartly.sdk; -import java8.util.concurrent.CompletableFuture; - import javax.annotation.Nonnull; -import com.absmartly.sdk.json.PublishEvent; - -public class DefaultContextEventHandler implements ContextEventHandler { +/** + * @deprecated Use {@link DefaultContextPublisher} instead. + */ +@Deprecated +public class DefaultContextEventHandler extends DefaultContextPublisher implements ContextEventHandler { public DefaultContextEventHandler(@Nonnull final Client client) { - client_ = client; + super(client); } - - @Override - public CompletableFuture publish(@Nonnull final Context context, @Nonnull final PublishEvent event) { - return client_.publish(event); - } - - private final Client client_; } diff --git a/core-api/src/main/java/com/absmartly/sdk/DefaultContextEventSerializer.java b/core-api/src/main/java/com/absmartly/sdk/DefaultContextEventSerializer.java index 6aba4fb..f729520 100644 --- a/core-api/src/main/java/com/absmartly/sdk/DefaultContextEventSerializer.java +++ b/core-api/src/main/java/com/absmartly/sdk/DefaultContextEventSerializer.java @@ -28,7 +28,7 @@ public byte[] serialize(@Nonnull final PublishEvent event) { try { return writer_.writeValueAsBytes(event); } catch (JsonProcessingException e) { - log.error("", e); + log.error("Failed to serialize publish event: {}", e.getMessage(), e); return null; } } diff --git a/core-api/src/main/java/com/absmartly/sdk/DefaultContextPublisher.java b/core-api/src/main/java/com/absmartly/sdk/DefaultContextPublisher.java new file mode 100644 index 0000000..0d4d7f2 --- /dev/null +++ b/core-api/src/main/java/com/absmartly/sdk/DefaultContextPublisher.java @@ -0,0 +1,20 @@ +package com.absmartly.sdk; + +import java8.util.concurrent.CompletableFuture; + +import javax.annotation.Nonnull; + +import com.absmartly.sdk.json.PublishEvent; + +public class DefaultContextPublisher implements ContextPublisher { + public DefaultContextPublisher(@Nonnull final Client client) { + client_ = client; + } + + @Override + public CompletableFuture publish(@Nonnull final Context context, @Nonnull final PublishEvent event) { + return client_.publish(event); + } + + private final Client client_; +} diff --git a/core-api/src/main/java/com/absmartly/sdk/DefaultVariableParser.java b/core-api/src/main/java/com/absmartly/sdk/DefaultVariableParser.java index 16375d8..9b6f67f 100644 --- a/core-api/src/main/java/com/absmartly/sdk/DefaultVariableParser.java +++ b/core-api/src/main/java/com/absmartly/sdk/DefaultVariableParser.java @@ -9,17 +9,18 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.fasterxml.jackson.databind.MapperFeature; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.ObjectReader; import com.fasterxml.jackson.databind.type.TypeFactory; +import com.absmartly.sdk.internal.JsonMapperUtils; + public class DefaultVariableParser implements VariableParser { private static final Logger log = LoggerFactory.getLogger(DefaultVariableParser.class); public DefaultVariableParser() { - final ObjectMapper objectMapper = new ObjectMapper(); - objectMapper.enable(MapperFeature.USE_STATIC_TYPING); + final ObjectMapper objectMapper = JsonMapperUtils.createStandardObjectMapper(); + this.reader_ = objectMapper .readerFor(TypeFactory.defaultInstance().constructMapType(HashMap.class, String.class, Object.class)); this.readerGeneric_ = objectMapper.readerFor(Object.class); @@ -30,7 +31,8 @@ public Map parse(@Nonnull final Context context, @Nonnull final try { return reader_.readValue(variableValues); } catch (IOException e) { - log.error("", e); + log.error("Failed to parse variable values for experiment '{}', variant '{}': {}", experimentName, + variantName, e.getMessage(), e); return null; } } @@ -40,7 +42,7 @@ public Object parse(@Nonnull final Context context, @Nonnull final String experi try { return readerGeneric_.readValue(variableValue); } catch (IOException e) { - log.error("", e); + log.error("Failed to parse custom field value for experiment '{}': {}", experimentName, e.getMessage(), e); return null; } } diff --git a/core-api/src/main/java/com/absmartly/sdk/deprecated/ABSmartly.java b/core-api/src/main/java/com/absmartly/sdk/deprecated/ABSmartly.java new file mode 100644 index 0000000..4135dde --- /dev/null +++ b/core-api/src/main/java/com/absmartly/sdk/deprecated/ABSmartly.java @@ -0,0 +1,19 @@ +package com.absmartly.sdk.deprecated; + +import javax.annotation.Nonnull; + +import com.absmartly.sdk.ABsmartly; + +/** + * @deprecated Use {@link com.absmartly.sdk.ABsmartly} instead. + */ +@Deprecated +public class ABSmartly extends ABsmartly { + public static ABSmartly create(@Nonnull ABSmartlyConfig config) { + return new ABSmartly(config); + } + + protected ABSmartly(@Nonnull ABSmartlyConfig config) { + super(config); + } +} diff --git a/core-api/src/main/java/com/absmartly/sdk/deprecated/ABSmartlyConfig.java b/core-api/src/main/java/com/absmartly/sdk/deprecated/ABSmartlyConfig.java new file mode 100644 index 0000000..a497c4f --- /dev/null +++ b/core-api/src/main/java/com/absmartly/sdk/deprecated/ABSmartlyConfig.java @@ -0,0 +1,76 @@ +package com.absmartly.sdk.deprecated; + +import java.util.concurrent.ScheduledExecutorService; + +import javax.annotation.Nonnull; + +import com.absmartly.sdk.ABsmartlyConfig; +import com.absmartly.sdk.AudienceDeserializer; +import com.absmartly.sdk.Client; +import com.absmartly.sdk.ContextDataProvider; +import com.absmartly.sdk.ContextEventHandler; +import com.absmartly.sdk.ContextEventLogger; +import com.absmartly.sdk.ContextPublisher; +import com.absmartly.sdk.VariableParser; + +/** + * @deprecated Use {@link com.absmartly.sdk.ABsmartlyConfig} instead. + */ +@Deprecated +public class ABSmartlyConfig extends ABsmartlyConfig { + public static ABSmartlyConfig create() { + return new ABSmartlyConfig(); + } + + protected ABSmartlyConfig() { + super(); + } + + @Override + public ABSmartlyConfig setContextDataProvider(@Nonnull final ContextDataProvider contextDataProvider) { + super.setContextDataProvider(contextDataProvider); + return this; + } + + @Override + public ABSmartlyConfig setContextPublisher(@Nonnull final ContextPublisher contextPublisher) { + super.setContextPublisher(contextPublisher); + return this; + } + + @Override + public ABSmartlyConfig setContextEventHandler(@Nonnull final ContextEventHandler contextEventHandler) { + super.setContextEventHandler(contextEventHandler); + return this; + } + + @Override + public ABSmartlyConfig setVariableParser(@Nonnull final VariableParser variableParser) { + super.setVariableParser(variableParser); + return this; + } + + @Override + public ABSmartlyConfig setScheduler(@Nonnull final ScheduledExecutorService scheduler) { + super.setScheduler(scheduler); + return this; + } + + @Override + public ABSmartlyConfig setContextEventLogger(@Nonnull final ContextEventLogger logger) { + super.setContextEventLogger(logger); + return this; + } + + @Override + public ABSmartlyConfig setAudienceDeserializer(@Nonnull final AudienceDeserializer audienceDeserializer) { + super.setAudienceDeserializer(audienceDeserializer); + return this; + } + + @Override + public ABSmartlyConfig setClient(Client client) { + super.setClient(client); + return this; + } +} diff --git a/core-api/src/main/java/com/absmartly/sdk/internal/Algorithm.java b/core-api/src/main/java/com/absmartly/sdk/internal/Algorithm.java index cf91a5b..33d3241 100644 --- a/core-api/src/main/java/com/absmartly/sdk/internal/Algorithm.java +++ b/core-api/src/main/java/com/absmartly/sdk/internal/Algorithm.java @@ -4,6 +4,7 @@ import java8.util.function.Function; public class Algorithm { + @SuppressWarnings("unchecked") public static R[] mapSetToArray(Set set, R[] array, Function mapper) { final int size = set.size(); if (array.length < size) { diff --git a/core-api/src/main/java/com/absmartly/sdk/internal/JsonMapperUtils.java b/core-api/src/main/java/com/absmartly/sdk/internal/JsonMapperUtils.java new file mode 100644 index 0000000..11e4a70 --- /dev/null +++ b/core-api/src/main/java/com/absmartly/sdk/internal/JsonMapperUtils.java @@ -0,0 +1,15 @@ +package com.absmartly.sdk.internal; + +import com.fasterxml.jackson.databind.DeserializationFeature; +import com.fasterxml.jackson.databind.MapperFeature; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.json.JsonMapper; + +public class JsonMapperUtils { + public static ObjectMapper createStandardObjectMapper() { + return JsonMapper.builder() + .enable(MapperFeature.USE_STATIC_TYPING) + .enable(DeserializationFeature.FAIL_ON_READING_DUP_TREE_KEY) + .build(); + } +} diff --git a/core-api/src/main/java/com/absmartly/sdk/java/time/Clock.java b/core-api/src/main/java/com/absmartly/sdk/java/time/Clock.java index aea0a45..2c9a972 100644 --- a/core-api/src/main/java/com/absmartly/sdk/java/time/Clock.java +++ b/core-api/src/main/java/com/absmartly/sdk/java/time/Clock.java @@ -11,12 +11,8 @@ static public Clock fixed(long millis) { } static public Clock systemUTC() { - if (utc_ != null) { - return utc_; - } - - return utc_ = new SystemClockUTC(); + return utc_; } - static SystemClockUTC utc_; + static final SystemClockUTC utc_ = new SystemClockUTC(); } diff --git a/core-api/src/main/java/com/absmartly/sdk/jsonexpr/ExprEvaluator.java b/core-api/src/main/java/com/absmartly/sdk/jsonexpr/ExprEvaluator.java index 119a7e9..975e400 100644 --- a/core-api/src/main/java/com/absmartly/sdk/jsonexpr/ExprEvaluator.java +++ b/core-api/src/main/java/com/absmartly/sdk/jsonexpr/ExprEvaluator.java @@ -6,7 +6,12 @@ import java.util.List; import java.util.Map; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + public class ExprEvaluator implements Evaluator { + private static final Logger log = LoggerFactory.getLogger(ExprEvaluator.class); + final static ThreadLocal formatter = new ThreadLocal() { @Override public DecimalFormat initialValue() { @@ -34,6 +39,10 @@ public Object evaluate(Object expr) { final Operator op = operators.get(entry.getKey()); if (op != null) { return op.evaluate(this, entry.getValue()); + } else { + log.warn( + "Unknown operator in audience expression: '{}'. This may be a forward compatibility issue with newer server versions.", + entry.getKey()); } break; } @@ -63,7 +72,7 @@ public Double numberConvert(Object x) { } else if (x instanceof String) { try { return Double.parseDouble((String) x); // use javascript semantics: numbers are doubles - } catch (Throwable ignored) {} + } catch (NumberFormatException ignored) {} } return null; @@ -93,7 +102,7 @@ public Object extractVar(String path) { final List list = (List) target; try { value = list.get(Integer.parseInt(frag)); - } catch (Throwable ignored) {} + } catch (NumberFormatException ignored) {} catch (IndexOutOfBoundsException ignored) {} } else if (target instanceof Map) { final Map map = (Map) target; value = map.get(frag); diff --git a/core-api/src/main/java/com/absmartly/sdk/jsonexpr/operators/EqualsOperator.java b/core-api/src/main/java/com/absmartly/sdk/jsonexpr/operators/EqualsOperator.java index 7406df1..5a3972e 100644 --- a/core-api/src/main/java/com/absmartly/sdk/jsonexpr/operators/EqualsOperator.java +++ b/core-api/src/main/java/com/absmartly/sdk/jsonexpr/operators/EqualsOperator.java @@ -1,11 +1,26 @@ package com.absmartly.sdk.jsonexpr.operators; +import java.util.List; + import com.absmartly.sdk.jsonexpr.Evaluator; +import com.absmartly.sdk.jsonexpr.Operator; -public class EqualsOperator extends BinaryOperator { +public class EqualsOperator implements Operator { @Override - public Object binary(Evaluator evaluator, Object lhs, Object rhs) { - final Integer result = evaluator.compare(lhs, rhs); - return (result != null) ? (result == 0) : null; + public Object evaluate(Evaluator evaluator, Object args) { + if (args instanceof List) { + final List argsList = (List) args; + final Object lhs = argsList.size() > 0 ? evaluator.evaluate(argsList.get(0)) : null; + final Object rhs = argsList.size() > 1 ? evaluator.evaluate(argsList.get(1)) : null; + if (lhs == null && rhs == null) { + return true; + } + if (lhs == null || rhs == null) { + return null; + } + final Integer result = evaluator.compare(lhs, rhs); + return (result != null) ? (result == 0) : null; + } + return null; } } diff --git a/core-api/src/main/java/com/absmartly/sdk/jsonexpr/operators/InOperator.java b/core-api/src/main/java/com/absmartly/sdk/jsonexpr/operators/InOperator.java index 6f4fdf2..ff3208f 100644 --- a/core-api/src/main/java/com/absmartly/sdk/jsonexpr/operators/InOperator.java +++ b/core-api/src/main/java/com/absmartly/sdk/jsonexpr/operators/InOperator.java @@ -7,10 +7,11 @@ public class InOperator extends BinaryOperator { @Override - public Object binary(Evaluator evaluator, Object haystack, Object needle) { + public Object binary(Evaluator evaluator, Object needle, Object haystack) { if (haystack instanceof List) { for (final Object item : (List) haystack) { - if (evaluator.compare(item, needle) == 0) { + final Integer cmp = evaluator.compare(item, needle); + if (cmp != null && cmp == 0) { return true; } } diff --git a/core-api/src/main/java/com/absmartly/sdk/jsonexpr/operators/MatchOperator.java b/core-api/src/main/java/com/absmartly/sdk/jsonexpr/operators/MatchOperator.java index 4f05858..4949490 100644 --- a/core-api/src/main/java/com/absmartly/sdk/jsonexpr/operators/MatchOperator.java +++ b/core-api/src/main/java/com/absmartly/sdk/jsonexpr/operators/MatchOperator.java @@ -1,24 +1,127 @@ package com.absmartly.sdk.jsonexpr.operators; +import java.util.concurrent.*; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.regex.PatternSyntaxException; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import com.absmartly.sdk.jsonexpr.Evaluator; public class MatchOperator extends BinaryOperator { + private static final Logger log = LoggerFactory.getLogger(MatchOperator.class); + private static final int MAX_PATTERN_LENGTH = 1000; + private static final int MAX_TEXT_LENGTH = 10000; + private static final int REGEX_TIMEOUT_MS = 100; + private static final ExecutorService REGEX_POOL = new ThreadPoolExecutor( + 0, 4, 60L, TimeUnit.SECONDS, + new SynchronousQueue(), + new DaemonThreadFactory(), + new ThreadPoolExecutor.CallerRunsPolicy()); + @Override public Object binary(Evaluator evaluator, Object lhs, Object rhs) { final String text = evaluator.stringConvert(lhs); if (text != null) { final String pattern = evaluator.stringConvert(rhs); if (pattern != null) { + if (pattern.length() > MAX_PATTERN_LENGTH) { + log.warn("Regex pattern exceeds maximum length of {}: {}", MAX_PATTERN_LENGTH, + pattern.substring(0, 50) + "..."); + return null; + } + + if (text.length() > MAX_TEXT_LENGTH) { + log.warn("Regex input text exceeds maximum length of {}", MAX_TEXT_LENGTH); + return null; + } + try { final Pattern compiled = Pattern.compile(pattern); - final Matcher matcher = compiled.matcher(text); - return matcher.find(); - } catch (Throwable ignored) {} + final InterruptibleCharSequence interruptible = new InterruptibleCharSequence(text); + + Future future = REGEX_POOL.submit(new Callable() { + @Override + public Boolean call() { + final Matcher matcher = compiled.matcher(interruptible); + return matcher.find(); + } + }); + + try { + return future.get(REGEX_TIMEOUT_MS, TimeUnit.MILLISECONDS); + } catch (TimeoutException e) { + future.cancel(true); + log.warn("Regex pattern timed out after {}ms, possible ReDoS attack: {}", REGEX_TIMEOUT_MS, + pattern); + return null; + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + return null; + } catch (ExecutionException e) { + Throwable cause = e.getCause(); + if (cause instanceof InterruptibleCharSequence.InterruptedCharAccessException) { + log.warn("Regex pattern interrupted after timeout, possible ReDoS attack: {}", pattern); + return null; + } + log.warn("Regex execution failed: {}", cause != null ? cause.getMessage() : e.getMessage()); + return null; + } + } catch (PatternSyntaxException e) { + log.warn("Invalid regex pattern from server: {}", e.getMessage()); + return null; + } } } return null; } + + static class InterruptibleCharSequence implements CharSequence { + private final CharSequence delegate; + + InterruptibleCharSequence(CharSequence delegate) { + this.delegate = delegate; + } + + @Override + public int length() { + return delegate.length(); + } + + @Override + public char charAt(int index) { + if (Thread.currentThread().isInterrupted()) { + throw new InterruptedCharAccessException(); + } + return delegate.charAt(index); + } + + @Override + public CharSequence subSequence(int start, int end) { + return new InterruptibleCharSequence(delegate.subSequence(start, end)); + } + + @Override + public String toString() { + return delegate.toString(); + } + + static class InterruptedCharAccessException extends RuntimeException { + InterruptedCharAccessException() { + super("CharSequence access interrupted"); + } + } + } + + private static class DaemonThreadFactory implements ThreadFactory { + @Override + public Thread newThread(Runnable r) { + Thread t = new Thread(r); + t.setDaemon(true); + t.setName("absmartly-regex-" + t.getId()); + return t; + } + } } diff --git a/core-api/src/test/java/com/absmartly/sdk/ABSmartlyConfigTest.java b/core-api/src/test/java/com/absmartly/sdk/ABsmartlyConfigTest.java similarity index 82% rename from core-api/src/test/java/com/absmartly/sdk/ABSmartlyConfigTest.java rename to core-api/src/test/java/com/absmartly/sdk/ABsmartlyConfigTest.java index 18d6f36..50c1150 100644 --- a/core-api/src/test/java/com/absmartly/sdk/ABSmartlyConfigTest.java +++ b/core-api/src/test/java/com/absmartly/sdk/ABsmartlyConfigTest.java @@ -7,39 +7,39 @@ import org.junit.jupiter.api.Test; -class ABSmartlyConfigTest extends TestUtils { +class ABsmartlyConfigTest extends TestUtils { @Test void setContextDataProvider() { final ContextDataProvider provider = mock(ContextDataProvider.class); - final ABSmartlyConfig config = ABSmartlyConfig.create().setContextDataProvider(provider); + final ABsmartlyConfig config = ABsmartlyConfig.create().setContextDataProvider(provider); assertSame(provider, config.getContextDataProvider()); } @Test void setContextEventHandler() { final ContextEventHandler handler = mock(ContextEventHandler.class); - final ABSmartlyConfig config = ABSmartlyConfig.create().setContextEventHandler(handler); + final ABsmartlyConfig config = ABsmartlyConfig.create().setContextEventHandler(handler); assertSame(handler, config.getContextEventHandler()); } @Test void setVariableParser() { final VariableParser variableParser = mock(VariableParser.class); - final ABSmartlyConfig config = ABSmartlyConfig.create().setVariableParser(variableParser); + final ABsmartlyConfig config = ABsmartlyConfig.create().setVariableParser(variableParser); assertSame(variableParser, config.getVariableParser()); } @Test void setScheduler() { final ScheduledExecutorService scheduler = mock(ScheduledExecutorService.class); - final ABSmartlyConfig config = ABSmartlyConfig.create().setScheduler(scheduler); + final ABsmartlyConfig config = ABsmartlyConfig.create().setScheduler(scheduler); assertSame(scheduler, config.getScheduler()); } @Test void setContextEventLogger() { final ContextEventLogger logger = mock(ContextEventLogger.class); - final ABSmartlyConfig config = ABSmartlyConfig.create().setContextEventLogger(logger); + final ABsmartlyConfig config = ABsmartlyConfig.create().setContextEventLogger(logger); assertSame(logger, config.getContextEventLogger()); } @@ -50,7 +50,7 @@ void setAll() { final VariableParser parser = mock(VariableParser.class); final ScheduledExecutorService scheduler = mock(ScheduledExecutorService.class); final Client client = mock(Client.class); - final ABSmartlyConfig config = ABSmartlyConfig.create() + final ABsmartlyConfig config = ABsmartlyConfig.create() .setVariableParser(parser) .setContextDataProvider(provider) .setContextEventHandler(handler) diff --git a/core-api/src/test/java/com/absmartly/sdk/ABsmartlyFixTest.java b/core-api/src/test/java/com/absmartly/sdk/ABsmartlyFixTest.java new file mode 100644 index 0000000..6c7c385 --- /dev/null +++ b/core-api/src/test/java/com/absmartly/sdk/ABsmartlyFixTest.java @@ -0,0 +1,79 @@ +package com.absmartly.sdk; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +import java.io.IOException; +import java.util.concurrent.ScheduledExecutorService; +import java8.util.concurrent.CompletableFuture; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; + +import com.absmartly.sdk.json.ContextData; + +class ABsmartlyFixTest extends TestUtils { + Client client; + + @BeforeEach + void setUp() { + client = mock(Client.class); + } + + @Test + void createContextThrowsAfterClose() throws IOException { + final ABsmartlyConfig config = ABsmartlyConfig.create() + .setClient(client); + + final ABsmartly absmartly = ABsmartly.create(config); + absmartly.close(); + + assertThrows(IllegalStateException.class, () -> { + absmartly.createContext(ContextConfig.create().setUnit("user_id", "123")); + }); + } + + @Test + void createContextWithThrowsAfterClose() throws IOException { + final ABsmartlyConfig config = ABsmartlyConfig.create() + .setClient(client); + + final ABsmartly absmartly = ABsmartly.create(config); + absmartly.close(); + + assertThrows(IllegalStateException.class, () -> { + absmartly.createContextWith(ContextConfig.create().setUnit("user_id", "123"), new ContextData()); + }); + } + + @Test + void getContextDataThrowsAfterClose() throws IOException { + final ContextDataProvider dataProvider = mock(ContextDataProvider.class); + when(dataProvider.getContextData()).thenReturn(mock(CompletableFuture.class)); + + final ABsmartlyConfig config = ABsmartlyConfig.create() + .setClient(client) + .setContextDataProvider(dataProvider); + + final ABsmartly absmartly = ABsmartly.create(config); + absmartly.close(); + + assertThrows(IllegalStateException.class, absmartly::getContextData); + } + + @Test + void closeIsIdempotent() throws IOException { + final ScheduledExecutorService scheduler = mock(ScheduledExecutorService.class); + + final ABsmartlyConfig config = ABsmartlyConfig.create() + .setClient(client) + .setScheduler(scheduler); + + final ABsmartly absmartly = ABsmartly.create(config); + absmartly.close(); + absmartly.close(); + + verify(client, times(1)).close(); + } +} diff --git a/core-api/src/test/java/com/absmartly/sdk/ABSmartlyTest.java b/core-api/src/test/java/com/absmartly/sdk/ABsmartlyTest.java similarity index 74% rename from core-api/src/test/java/com/absmartly/sdk/ABSmartlyTest.java rename to core-api/src/test/java/com/absmartly/sdk/ABsmartlyTest.java index 7b5e7a2..577a193 100644 --- a/core-api/src/test/java/com/absmartly/sdk/ABSmartlyTest.java +++ b/core-api/src/test/java/com/absmartly/sdk/ABsmartlyTest.java @@ -18,7 +18,7 @@ import com.absmartly.sdk.java.time.Clock; import com.absmartly.sdk.json.ContextData; -class ABSmartlyTest extends TestUtils { +class ABsmartlyTest extends TestUtils { Client client; @BeforeEach @@ -28,25 +28,95 @@ void setUp() { @Test void create() { - final ABSmartlyConfig config = ABSmartlyConfig.create() + final ABsmartlyConfig config = ABsmartlyConfig.create() .setClient(client); - final ABSmartly absmartly = ABSmartly.create(config); + final ABsmartly absmartly = ABsmartly.create(config); assertNotNull(absmartly); } @Test void createThrowsWithInvalidConfig() { assertThrows(IllegalArgumentException.class, () -> { - final ABSmartlyConfig config = ABSmartlyConfig.create(); + final ABsmartlyConfig config = ABsmartlyConfig.create(); - final ABSmartly absmartly = ABSmartly.create(config); + final ABsmartly absmartly = ABsmartly.create(config); }, "Missing Client instance configuration"); } + @Test + void builderCreatesWithConnectionParams() { + try (final MockedStatic clientStatic = mockStatic(Client.class); + final MockedConstruction dataProviderCtor = mockConstruction( + DefaultContextDataProvider.class)) { + clientStatic.when(() -> Client.create(any(ClientConfig.class))).thenReturn(client); + + final ABsmartly absmartly = ABsmartly.builder() + .endpoint("https://test.absmartly.io/v1") + .apiKey("test-api-key") + .application("website") + .environment("production") + .build(); + assertNotNull(absmartly); + + final ArgumentCaptor clientConfigCaptor = ArgumentCaptor.forClass(ClientConfig.class); + clientStatic.verify(() -> Client.create(clientConfigCaptor.capture()), Mockito.times(1)); + + final ClientConfig capturedClientConfig = clientConfigCaptor.getValue(); + assertEquals("https://test.absmartly.io/v1", capturedClientConfig.getEndpoint()); + assertEquals("test-api-key", capturedClientConfig.getAPIKey()); + assertEquals("website", capturedClientConfig.getApplication()); + assertEquals("production", capturedClientConfig.getEnvironment()); + } + } + + @Test + void builderThrowsWithMissingEndpoint() { + assertThrows(IllegalArgumentException.class, () -> { + ABsmartly.builder() + .apiKey("test-api-key") + .application("website") + .environment("production") + .build(); + }); + } + + @Test + void builderThrowsWithMissingApiKey() { + assertThrows(IllegalArgumentException.class, () -> { + ABsmartly.builder() + .endpoint("https://test.absmartly.io/v1") + .application("website") + .environment("production") + .build(); + }); + } + + @Test + void builderThrowsWithMissingApplication() { + assertThrows(IllegalArgumentException.class, () -> { + ABsmartly.builder() + .endpoint("https://test.absmartly.io/v1") + .apiKey("test-api-key") + .environment("production") + .build(); + }); + } + + @Test + void builderThrowsWithMissingEnvironment() { + assertThrows(IllegalArgumentException.class, () -> { + ABsmartly.builder() + .endpoint("https://test.absmartly.io/v1") + .apiKey("test-api-key") + .application("website") + .build(); + }); + } + @Test void createContext() { - final ABSmartlyConfig config = ABSmartlyConfig.create() + final ABsmartlyConfig config = ABsmartlyConfig.create() .setClient(client); final CompletableFuture dataFuture = (CompletableFuture) mock( @@ -55,7 +125,7 @@ void createContext() { DefaultContextDataProvider.class, (mock, context) -> { when(mock.getContextData()).thenReturn(dataFuture); })) { - final ABSmartly absmartly = ABSmartly.create(config); + final ABsmartly absmartly = ABsmartly.create(config); assertEquals(1, dataProviderCtor.constructed().size()); try (final MockedStatic contextStatic = mockStatic(Context.class)) { @@ -75,8 +145,8 @@ void createContext() { .forClass(CompletableFuture.class); final ArgumentCaptor dataProviderCaptor = ArgumentCaptor .forClass(ContextDataProvider.class); - final ArgumentCaptor eventHandlerCaptor = ArgumentCaptor - .forClass(ContextEventHandler.class); + final ArgumentCaptor eventHandlerCaptor = ArgumentCaptor + .forClass(ContextPublisher.class); final ArgumentCaptor eventLoggerCaptor = ArgumentCaptor .forClass(ContextEventLogger.class); final ArgumentCaptor variableParserCaptor = ArgumentCaptor @@ -84,20 +154,22 @@ void createContext() { final ArgumentCaptor audienceMatcherCaptor = ArgumentCaptor .forClass(AudienceMatcher.class); - contextStatic.verify(Mockito.timeout(5000).times(1), - () -> Context.create(any(), any(), any(), any(), any(), any(), any(), any(), any())); - contextStatic.verify(Mockito.timeout(5000).times(1), + contextStatic.verify( + () -> Context.create(any(), any(), any(), any(), any(), any(), any(), any(), any()), + Mockito.times(1)); + contextStatic.verify( () -> Context.create(clockCaptor.capture(), configCaptor.capture(), schedulerCaptor.capture(), dataFutureCaptor.capture(), dataProviderCaptor.capture(), eventHandlerCaptor.capture(), eventLoggerCaptor.capture(), variableParserCaptor.capture(), - audienceMatcherCaptor.capture())); + audienceMatcherCaptor.capture()), + Mockito.times(1)); assertEquals(Clock.systemUTC(), clockCaptor.getValue()); assertSame(contextConfig, configCaptor.getValue()); assertTrue(schedulerCaptor.getValue() instanceof ScheduledThreadPoolExecutor); assertSame(dataFuture, dataFutureCaptor.getValue()); assertTrue(dataProviderCaptor.getValue() instanceof DefaultContextDataProvider); - assertTrue(eventHandlerCaptor.getValue() instanceof DefaultContextEventHandler); + assertTrue(eventHandlerCaptor.getValue() instanceof DefaultContextPublisher); assertNull(eventLoggerCaptor.getValue()); assertTrue(variableParserCaptor.getValue() instanceof DefaultVariableParser); assertNotNull(audienceMatcherCaptor.getValue()); @@ -107,13 +179,13 @@ void createContext() { @Test void createContextWith() { - final ABSmartlyConfig config = ABSmartlyConfig.create() + final ABsmartlyConfig config = ABsmartlyConfig.create() .setClient(client); final ContextData data = new ContextData(); try (final MockedConstruction dataProviderCtor = mockConstruction( DefaultContextDataProvider.class)) { - final ABSmartly absmartly = ABSmartly.create(config); + final ABsmartly absmartly = ABsmartly.create(config); assertEquals(1, dataProviderCtor.constructed().size()); try (final MockedStatic contextStatic = mockStatic(Context.class)) { @@ -135,8 +207,8 @@ void createContextWith() { .forClass(CompletableFuture.class); final ArgumentCaptor dataProviderCaptor = ArgumentCaptor .forClass(ContextDataProvider.class); - final ArgumentCaptor eventHandlerCaptor = ArgumentCaptor - .forClass(ContextEventHandler.class); + final ArgumentCaptor eventHandlerCaptor = ArgumentCaptor + .forClass(ContextPublisher.class); final ArgumentCaptor eventLoggerCaptor = ArgumentCaptor .forClass(ContextEventLogger.class); final ArgumentCaptor variableParserCaptor = ArgumentCaptor @@ -144,20 +216,22 @@ void createContextWith() { final ArgumentCaptor audienceMatcherCaptor = ArgumentCaptor .forClass(AudienceMatcher.class); - contextStatic.verify(Mockito.timeout(5000).times(1), - () -> Context.create(any(), any(), any(), any(), any(), any(), any(), any(), any())); - contextStatic.verify(Mockito.timeout(5000).times(1), + contextStatic.verify( + () -> Context.create(any(), any(), any(), any(), any(), any(), any(), any(), any()), + Mockito.times(1)); + contextStatic.verify( () -> Context.create(clockCaptor.capture(), configCaptor.capture(), schedulerCaptor.capture(), dataFutureCaptor.capture(), dataProviderCaptor.capture(), eventHandlerCaptor.capture(), eventLoggerCaptor.capture(), variableParserCaptor.capture(), - audienceMatcherCaptor.capture())); + audienceMatcherCaptor.capture()), + Mockito.times(1)); assertEquals(Clock.systemUTC(), clockCaptor.getValue()); assertSame(contextConfig, configCaptor.getValue()); assertTrue(schedulerCaptor.getValue() instanceof ScheduledThreadPoolExecutor); assertDoesNotThrow(() -> assertSame(data, dataFutureCaptor.getValue().get())); assertTrue(dataProviderCaptor.getValue() instanceof DefaultContextDataProvider); - assertTrue(eventHandlerCaptor.getValue() instanceof DefaultContextEventHandler); + assertTrue(eventHandlerCaptor.getValue() instanceof DefaultContextPublisher); assertNull(eventLoggerCaptor.getValue()); assertTrue(variableParserCaptor.getValue() instanceof DefaultVariableParser); assertNotNull(audienceMatcherCaptor.getValue()); @@ -171,11 +245,11 @@ void getContextData() { final ContextDataProvider dataProvider = mock(ContextDataProvider.class); when(dataProvider.getContextData()).thenReturn(dataFuture); - final ABSmartlyConfig config = ABSmartlyConfig.create() + final ABsmartlyConfig config = ABsmartlyConfig.create() .setClient(client) .setContextDataProvider(dataProvider); - final ABSmartly absmartly = ABSmartly.create(config); + final ABsmartly absmartly = ABsmartly.create(config); final CompletableFuture contextDataFuture = absmartly.getContextData(); verify(dataProvider, Mockito.timeout(5000).times(1)).getContextData(); @@ -195,7 +269,7 @@ void createContextWithCustomImpls() { final AudienceDeserializer audienceDeserializer = mock(AudienceDeserializer.class); final VariableParser variableParser = mock(VariableParser.class); - final ABSmartlyConfig config = ABSmartlyConfig.create() + final ABsmartlyConfig config = ABsmartlyConfig.create() .setClient(client) .setContextDataProvider(dataProvider) .setContextEventHandler(eventHandler) @@ -204,7 +278,7 @@ void createContextWithCustomImpls() { .setAudienceDeserializer(audienceDeserializer) .setVariableParser(variableParser); - final ABSmartly absmartly = ABSmartly.create(config); + final ABsmartly absmartly = ABsmartly.create(config); try (final MockedStatic contextStatic = mockStatic(Context.class); final MockedConstruction audienceMatcherCtor = mockConstruction(AudienceMatcher.class, @@ -228,19 +302,21 @@ void createContextWithCustomImpls() { .forClass(CompletableFuture.class); final ArgumentCaptor dataProviderCaptor = ArgumentCaptor .forClass(ContextDataProvider.class); - final ArgumentCaptor eventHandlerCaptor = ArgumentCaptor - .forClass(ContextEventHandler.class); + final ArgumentCaptor eventHandlerCaptor = ArgumentCaptor + .forClass(ContextPublisher.class); final ArgumentCaptor eventLoggerCaptor = ArgumentCaptor .forClass(ContextEventLogger.class); final ArgumentCaptor variableParserCaptor = ArgumentCaptor.forClass(VariableParser.class); final ArgumentCaptor audienceMatcher = ArgumentCaptor.forClass(AudienceMatcher.class); - contextStatic.verify(Mockito.timeout(5000).times(1), - () -> Context.create(any(), any(), any(), any(), any(), any(), any(), any(), any())); - contextStatic.verify(Mockito.timeout(5000).times(1), + contextStatic.verify( + () -> Context.create(any(), any(), any(), any(), any(), any(), any(), any(), any()), + Mockito.times(1)); + contextStatic.verify( () -> Context.create(clockCaptor.capture(), configCaptor.capture(), schedulerCaptor.capture(), dataFutureCaptor.capture(), dataProviderCaptor.capture(), eventHandlerCaptor.capture(), - eventLoggerCaptor.capture(), variableParserCaptor.capture(), audienceMatcher.capture())); + eventLoggerCaptor.capture(), variableParserCaptor.capture(), audienceMatcher.capture()), + Mockito.times(1)); assertEquals(Clock.systemUTC(), clockCaptor.getValue()); assertSame(contextConfig, configCaptor.getValue()); @@ -258,11 +334,11 @@ void createContextWithCustomImpls() { void close() throws IOException, InterruptedException { final ScheduledExecutorService scheduler = mock(ScheduledExecutorService.class); - final ABSmartlyConfig config = ABSmartlyConfig.create() + final ABsmartlyConfig config = ABsmartlyConfig.create() .setClient(client) .setScheduler(scheduler); - final ABSmartly absmartly = ABSmartly.create(config); + final ABsmartly absmartly = ABsmartly.create(config); try (final MockedStatic contextStatic = mockStatic(Context.class)) { final Context contextMock = mock(Context.class); diff --git a/core-api/src/test/java/com/absmartly/sdk/ClientConfigTest.java b/core-api/src/test/java/com/absmartly/sdk/ClientConfigTest.java index d571c01..729dba2 100644 --- a/core-api/src/test/java/com/absmartly/sdk/ClientConfigTest.java +++ b/core-api/src/test/java/com/absmartly/sdk/ClientConfigTest.java @@ -1,7 +1,6 @@ package com.absmartly.sdk; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.mock; import java.util.Properties; @@ -103,4 +102,80 @@ void createFromProperties() { assertSame(serializer, config.getContextEventSerializer()); assertSame(executor, config.getExecutor()); } + + @Test + void testEmptyEndpointUrl() { + final ClientConfig config = ClientConfig.create() + .setEndpoint("") + .setAPIKey("api-key-test") + .setEnvironment("test") + .setApplication("website"); + + assertEquals("", config.getEndpoint()); + } + + @Test + void testNullEndpointUrl() { + final ClientConfig config = ClientConfig.create() + .setAPIKey("api-key-test") + .setEnvironment("test") + .setApplication("website"); + + assertNull(config.getEndpoint()); + } + + @Test + void testEmptyApiKey() { + final ClientConfig config = ClientConfig.create() + .setEndpoint("https://test.endpoint.com") + .setAPIKey("") + .setEnvironment("test") + .setApplication("website"); + + assertEquals("", config.getAPIKey()); + } + + @Test + void testNullApiKey() { + final ClientConfig config = ClientConfig.create() + .setEndpoint("https://test.endpoint.com") + .setEnvironment("test") + .setApplication("website"); + + assertNull(config.getAPIKey()); + } + + @Test + void testMissingPropertiesPrefix() { + final Properties props = new Properties(); + props.putAll(TestUtils.mapOf( + "endpoint", "https://test.endpoint.com", + "environment", "test", + "apikey", "api-key-test", + "application", "website")); + + final ClientConfig config = ClientConfig.createFromProperties(props, "absmartly."); + + assertNull(config.getEndpoint()); + assertNull(config.getAPIKey()); + assertNull(config.getEnvironment()); + assertNull(config.getApplication()); + } + + @Test + void testCreateFromPropertiesWithEmptyPrefix() { + final Properties props = new Properties(); + props.putAll(TestUtils.mapOf( + "endpoint", "https://test.endpoint.com", + "environment", "test", + "apikey", "api-key-test", + "application", "website")); + + final ClientConfig config = ClientConfig.createFromProperties(props); + + assertEquals("https://test.endpoint.com", config.getEndpoint()); + assertEquals("api-key-test", config.getAPIKey()); + assertEquals("test", config.getEnvironment()); + assertEquals("website", config.getApplication()); + } } diff --git a/core-api/src/test/java/com/absmartly/sdk/ClientFixTest.java b/core-api/src/test/java/com/absmartly/sdk/ClientFixTest.java new file mode 100644 index 0000000..0fbbc71 --- /dev/null +++ b/core-api/src/test/java/com/absmartly/sdk/ClientFixTest.java @@ -0,0 +1,153 @@ +package com.absmartly.sdk; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.*; + +import java.util.Map; +import java8.util.concurrent.CompletableFuture; +import java8.util.concurrent.CompletionException; + +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +import com.absmartly.sdk.java.nio.charset.StandardCharsets; +import com.absmartly.sdk.json.ContextData; + +class ClientFixTest extends TestUtils { + + @Test + void endpointTrailingSlashNormalized() { + final HTTPClient httpClient = mock(HTTPClient.class); + final ContextDataDeserializer deser = mock(ContextDataDeserializer.class); + final Client client = Client.create(ClientConfig.create() + .setEndpoint("https://localhost/v1/") + .setAPIKey("test-api-key") + .setApplication("website") + .setEnvironment("dev") + .setContextDataDeserializer(deser), httpClient); + + final byte[] bytes = "{}".getBytes(StandardCharsets.UTF_8); + final ContextData expected = new ContextData(); + + final Map expectedQuery = mapOf( + "application", "website", + "environment", "dev"); + + when(httpClient.get("https://localhost/v1/context", expectedQuery, null)) + .thenReturn(CompletableFuture.completedFuture(new DefaultHTTPClient.DefaultResponse(200, "OK", + "application/json", bytes))); + when(deser.deserialize(bytes, 0, bytes.length)).thenReturn(expected); + + final CompletableFuture dataFuture = client.getContextData(); + final ContextData actual = dataFuture.join(); + + assertSame(expected, actual); + verify(httpClient, Mockito.timeout(5000).times(1)).get("https://localhost/v1/context", expectedQuery, null); + } + + @Test + void endpointWithoutTrailingSlashUnchanged() { + final HTTPClient httpClient = mock(HTTPClient.class); + final ContextDataDeserializer deser = mock(ContextDataDeserializer.class); + final Client client = Client.create(ClientConfig.create() + .setEndpoint("https://localhost/v1") + .setAPIKey("test-api-key") + .setApplication("website") + .setEnvironment("dev") + .setContextDataDeserializer(deser), httpClient); + + final byte[] bytes = "{}".getBytes(StandardCharsets.UTF_8); + final ContextData expected = new ContextData(); + + final Map expectedQuery = mapOf( + "application", "website", + "environment", "dev"); + + when(httpClient.get("https://localhost/v1/context", expectedQuery, null)) + .thenReturn(CompletableFuture.completedFuture(new DefaultHTTPClient.DefaultResponse(200, "OK", + "application/json", bytes))); + when(deser.deserialize(bytes, 0, bytes.length)).thenReturn(expected); + + final CompletableFuture dataFuture = client.getContextData(); + final ContextData actual = dataFuture.join(); + + assertSame(expected, actual); + } + + @Test + void httpEndpointDoesNotThrowButLogsWarning() { + final HTTPClient httpClient = mock(HTTPClient.class); + assertDoesNotThrow(() -> { + Client.create(ClientConfig.create() + .setEndpoint("http://localhost/v1") + .setAPIKey("test-api-key") + .setApplication("website") + .setEnvironment("dev"), httpClient); + }); + } + + @Test + void invalidProtocolThrows() { + final HTTPClient httpClient = mock(HTTPClient.class); + assertThrows(IllegalArgumentException.class, () -> { + Client.create(ClientConfig.create() + .setEndpoint("ftp://localhost/v1") + .setAPIKey("test-api-key") + .setApplication("website") + .setEnvironment("dev"), httpClient); + }); + } + + @Test + void getContextDataNullDeserializationThrowsExceptionally() { + final HTTPClient httpClient = mock(HTTPClient.class); + final ContextDataDeserializer deser = mock(ContextDataDeserializer.class); + final Client client = Client.create(ClientConfig.create() + .setEndpoint("https://localhost/v1") + .setAPIKey("test-api-key") + .setApplication("website") + .setEnvironment("dev") + .setContextDataDeserializer(deser), httpClient); + + final byte[] bytes = "invalid".getBytes(StandardCharsets.UTF_8); + + final Map expectedQuery = mapOf( + "application", "website", + "environment", "dev"); + + when(httpClient.get("https://localhost/v1/context", expectedQuery, null)) + .thenReturn(CompletableFuture.completedFuture(new DefaultHTTPClient.DefaultResponse(200, "OK", + "application/json", bytes))); + when(deser.deserialize(bytes, 0, bytes.length)).thenReturn(null); + + final CompletableFuture dataFuture = client.getContextData(); + final CompletionException actual = assertThrows(CompletionException.class, dataFuture::join); + assertTrue(actual.getCause() instanceof IllegalStateException); + assertEquals("Failed to deserialize context data response", actual.getCause().getMessage()); + } + + @Test + void getContextDataEmptyResponseThrowsExceptionally() { + final HTTPClient httpClient = mock(HTTPClient.class); + final ContextDataDeserializer deser = mock(ContextDataDeserializer.class); + final Client client = Client.create(ClientConfig.create() + .setEndpoint("https://localhost/v1") + .setAPIKey("test-api-key") + .setApplication("website") + .setEnvironment("dev") + .setContextDataDeserializer(deser), httpClient); + + final Map expectedQuery = mapOf( + "application", "website", + "environment", "dev"); + + when(httpClient.get("https://localhost/v1/context", expectedQuery, null)) + .thenReturn(CompletableFuture.completedFuture(new DefaultHTTPClient.DefaultResponse(200, "OK", + "application/json", new byte[0]))); + + final CompletableFuture dataFuture = client.getContextData(); + final CompletionException actual = assertThrows(CompletionException.class, dataFuture::join); + assertTrue(actual.getCause() instanceof IllegalStateException); + } +} diff --git a/core-api/src/test/java/com/absmartly/sdk/ContextFixTest.java b/core-api/src/test/java/com/absmartly/sdk/ContextFixTest.java new file mode 100644 index 0000000..845b659 --- /dev/null +++ b/core-api/src/test/java/com/absmartly/sdk/ContextFixTest.java @@ -0,0 +1,105 @@ +package com.absmartly.sdk; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +import java.util.concurrent.ScheduledExecutorService; +import java8.util.concurrent.CompletableFuture; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import com.absmartly.sdk.java.time.Clock; +import com.absmartly.sdk.json.ContextData; +import com.absmartly.sdk.json.Experiment; + +class ContextFixTest extends TestUtils { + + ContextDataProvider dataProvider; + ContextEventLogger eventLogger; + ContextEventHandler eventHandler; + VariableParser variableParser; + AudienceMatcher audienceMatcher; + ScheduledExecutorService scheduler; + Clock clock = Clock.fixed(1_620_000_000_000L); + + @BeforeEach + void setUp() { + dataProvider = mock(ContextDataProvider.class); + eventHandler = mock(ContextEventHandler.class); + eventLogger = mock(ContextEventLogger.class); + variableParser = new DefaultVariableParser(); + audienceMatcher = new AudienceMatcher(new DefaultAudienceDeserializer()); + scheduler = mock(ScheduledExecutorService.class); + } + + Context createReadyContext(ContextData data) { + final ContextConfig config = ContextConfig.create() + .setUnit("session_id", "e791e240fcd3df7d238cfc285f475e8152fcc0ec"); + + return Context.create(clock, config, scheduler, CompletableFuture.completedFuture(data), dataProvider, + eventHandler, eventLogger, variableParser, audienceMatcher); + } + + @Test + void setDataWithNullVariantsDoesNotThrowNPE() { + final ContextData data = new ContextData(); + final Experiment experiment = new Experiment(); + experiment.id = 1; + experiment.name = "exp_test"; + experiment.unitType = "session_id"; + experiment.variants = null; + data.experiments = new Experiment[]{experiment}; + + assertDoesNotThrow(() -> createReadyContext(data)); + } + + @Test + void setDataWithNullCustomFieldValuesDoesNotThrowNPE() { + final ContextData data = new ContextData(); + final Experiment experiment = new Experiment(); + experiment.id = 1; + experiment.name = "exp_test"; + experiment.unitType = "session_id"; + experiment.variants = new com.absmartly.sdk.json.ExperimentVariant[0]; + experiment.customFieldValues = null; + data.experiments = new Experiment[]{experiment}; + + assertDoesNotThrow(() -> createReadyContext(data)); + } + + @Test + void brandingInErrorMessages() { + final ContextData data = new ContextData(); + data.experiments = new Experiment[0]; + + final Context context = createReadyContext(data); + + when(eventHandler.publish(any(), any())).thenReturn(CompletableFuture.completedFuture(null)); + context.close(); + + IllegalStateException ex = assertThrows(IllegalStateException.class, + () -> context.setAttribute("test", "value")); + assertTrue(ex.getMessage().contains("ABsmartly")); + assertFalse(ex.getMessage().contains("ABSmartly")); + } + + @Test + void setDataWithNullExperimentVariantsReturnsDefaultTreatment() { + final ContextData data = new ContextData(); + final Experiment experiment = new Experiment(); + experiment.id = 1; + experiment.name = "exp_test"; + experiment.unitType = "session_id"; + experiment.variants = null; + experiment.trafficSplit = new double[]{1.0}; + experiment.split = new double[]{1.0}; + data.experiments = new Experiment[]{experiment}; + + final Context context = createReadyContext(data); + assertTrue(context.isReady()); + + int treatment = context.getTreatment("exp_test"); + assertEquals(0, treatment); + } +} diff --git a/core-api/src/test/java/com/absmartly/sdk/ContextTest.java b/core-api/src/test/java/com/absmartly/sdk/ContextTest.java index 0abe1c0..ed4d40b 100644 --- a/core-api/src/test/java/com/absmartly/sdk/ContextTest.java +++ b/core-api/src/test/java/com/absmartly/sdk/ContextTest.java @@ -4,6 +4,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -229,6 +230,29 @@ void becomesReadyAndFailedWithException() { assertTrue(context.isFailed()); } + @Test + void readyErrorReturnsNullOnSuccess() { + final Context context = createReadyContext(); + assertNull(context.readyError()); + } + + @Test + void readyErrorReturnsExceptionOnFailedFuture() { + final Context context = createContext(dataFutureFailed); + assertTrue(context.isFailed()); + assertNotNull(context.readyError()); + } + + @Test + void readyErrorReturnsExceptionOnAsyncFailure() { + final Context context = createContext(dataFuture); + final Exception error = new Exception("FAILED"); + dataFuture.completeExceptionally(error); + context.waitUntilReady(); + assertTrue(context.isFailed()); + assertNotNull(context.readyError()); + } + @Test void callsEventLoggerWhenReady() { final Context context = createContext(dataFuture); @@ -321,21 +345,34 @@ void throwsWhenNotReady() { assertFalse(context.isReady()); assertFalse(context.isFailed()); - final String notReadyMessage = "ABSmartly Context is not yet ready"; - assertEquals(notReadyMessage, - assertThrows(IllegalStateException.class, () -> context.peekTreatment("exp_test_ab")).getMessage()); - assertEquals(notReadyMessage, - assertThrows(IllegalStateException.class, () -> context.getTreatment("exp_test_ab")).getMessage()); + final String notReadyMessage = "ABsmartly Context is not yet ready."; assertEquals(notReadyMessage, assertThrows(IllegalStateException.class, context::getData).getMessage()); - assertEquals(notReadyMessage, assertThrows(IllegalStateException.class, context::getExperiments).getMessage()); - assertEquals(notReadyMessage, - assertThrows(IllegalStateException.class, () -> context.getVariableValue("banner.border", 17)) - .getMessage()); - assertEquals(notReadyMessage, - assertThrows(IllegalStateException.class, () -> context.peekVariableValue("banner.border", 17)) - .getMessage()); - assertEquals(notReadyMessage, - assertThrows(IllegalStateException.class, context::getVariableKeys).getMessage()); + + assertEquals(0, context.peekTreatment("exp_test_ab")); + assertEquals(0, context.getTreatment("exp_test_ab")); + assertArrayEquals(new String[0], context.getExperiments()); + assertEquals(17, context.getVariableValue("banner.border", 17)); + assertEquals(17, context.peekVariableValue("banner.border", 17)); + assertEquals(new HashMap<>(), context.getVariableKeys()); + assertArrayEquals(new String[0], context.getCustomFieldKeys()); + assertNull(context.getCustomFieldValue("exp_test_ab", "key")); + assertNull(context.getCustomFieldValueType("exp_test_ab", "key")); + } + + @Test + void returnsDefaultsWhenNotReady() { + final Context context = createContext(dataFuture); + assertFalse(context.isReady()); + + assertEquals(0, context.peekTreatment("exp_test_ab")); + assertEquals(0, context.getTreatment("exp_test_ab")); + assertArrayEquals(new String[0], context.getExperiments()); + assertEquals("default", context.getVariableValue("banner.border", "default")); + assertEquals("default", context.peekVariableValue("banner.border", "default")); + assertTrue(context.getVariableKeys().isEmpty()); + assertArrayEquals(new String[0], context.getCustomFieldKeys()); + assertNull(context.getCustomFieldValue("exp_test_ab", "key")); + assertNull(context.getCustomFieldValueType("exp_test_ab", "key")); } @Test @@ -354,17 +391,12 @@ void throwsWhenClosing() { assertTrue(context.isClosing()); assertFalse(context.isClosed()); - final String closingMessage = "ABSmartly Context is closing"; + final String closingMessage = "ABsmartly Context is closing."; assertEquals(closingMessage, assertThrows(IllegalStateException.class, () -> context.setAttribute("attr1", "value1")).getMessage()); assertEquals(closingMessage, assertThrows(IllegalStateException.class, () -> context.setAttributes(mapOf("attr1", "value1"))) - .getMessage()); - assertEquals(closingMessage, - assertThrows(IllegalStateException.class, () -> context.setOverride("exp_test_ab", 2)).getMessage()); - assertEquals(closingMessage, - assertThrows(IllegalStateException.class, () -> context.setOverrides(mapOf("exp_test_ab", 2))) .getMessage()); assertEquals(closingMessage, assertThrows(IllegalStateException.class, () -> context.setUnit("test", "test")) @@ -375,24 +407,21 @@ void throwsWhenClosing() { assertEquals(closingMessage, assertThrows(IllegalStateException.class, () -> context.setCustomAssignments(mapOf("exp_test_ab", 2))) - .getMessage()); - assertEquals(closingMessage, - assertThrows(IllegalStateException.class, () -> context.peekTreatment("exp_test_ab")).getMessage()); - assertEquals(closingMessage, - assertThrows(IllegalStateException.class, () -> context.getTreatment("exp_test_ab")).getMessage()); + .getMessage()); assertEquals(closingMessage, assertThrows(IllegalStateException.class, () -> context.track("goal1", null)).getMessage()); assertEquals(closingMessage, assertThrows(IllegalStateException.class, context::publish).getMessage()); assertEquals(closingMessage, assertThrows(IllegalStateException.class, context::getData).getMessage()); - assertEquals(closingMessage, assertThrows(IllegalStateException.class, context::getExperiments).getMessage()); - assertEquals(closingMessage, - assertThrows(IllegalStateException.class, () -> context.getVariableValue("banner.border", 17)) - .getMessage()); - assertEquals(closingMessage, - assertThrows(IllegalStateException.class, () -> context.peekVariableValue("banner.border", 17)) - .getMessage()); - assertEquals(closingMessage, - assertThrows(IllegalStateException.class, context::getVariableKeys).getMessage()); + + assertEquals(0, context.peekTreatment("exp_test_ab")); + assertEquals(0, context.getTreatment("exp_test_ab")); + assertArrayEquals(new String[0], context.getExperiments()); + assertEquals(17, context.getVariableValue("banner.border", 17)); + assertEquals(17, context.peekVariableValue("banner.border", 17)); + assertEquals(new HashMap<>(), context.getVariableKeys()); + assertArrayEquals(new String[0], context.getCustomFieldKeys()); + assertNull(context.getCustomFieldValue("exp_test_ab", "key")); + assertNull(context.getCustomFieldValueType("exp_test_ab", "key")); } @Test @@ -410,17 +439,12 @@ void throwsWhenClosed() { assertFalse(context.isClosing()); assertTrue(context.isClosed()); - final String closedMessage = "ABSmartly Context is closed"; + final String closedMessage = "ABsmartly Context is finalized."; assertEquals(closedMessage, assertThrows(IllegalStateException.class, () -> context.setAttribute("attr1", "value1")).getMessage()); assertEquals(closedMessage, assertThrows(IllegalStateException.class, () -> context.setAttributes(mapOf("attr1", "value1"))) - .getMessage()); - assertEquals(closedMessage, - assertThrows(IllegalStateException.class, () -> context.setOverride("exp_test_ab", 2)).getMessage()); - assertEquals(closedMessage, - assertThrows(IllegalStateException.class, () -> context.setOverrides(mapOf("exp_test_ab", 2))) .getMessage()); assertEquals(closedMessage, assertThrows(IllegalStateException.class, () -> context.setUnit("test", "test")) @@ -431,24 +455,56 @@ void throwsWhenClosed() { assertEquals(closedMessage, assertThrows(IllegalStateException.class, () -> context.setCustomAssignments(mapOf("exp_test_ab", 2))) - .getMessage()); - assertEquals(closedMessage, - assertThrows(IllegalStateException.class, () -> context.peekTreatment("exp_test_ab")).getMessage()); - assertEquals(closedMessage, - assertThrows(IllegalStateException.class, () -> context.getTreatment("exp_test_ab")).getMessage()); + .getMessage()); assertEquals(closedMessage, assertThrows(IllegalStateException.class, () -> context.track("goal1", null)).getMessage()); assertEquals(closedMessage, assertThrows(IllegalStateException.class, context::publish).getMessage()); assertEquals(closedMessage, assertThrows(IllegalStateException.class, context::getData).getMessage()); - assertEquals(closedMessage, assertThrows(IllegalStateException.class, context::getExperiments).getMessage()); - assertEquals(closedMessage, - assertThrows(IllegalStateException.class, () -> context.getVariableValue("banner.border", 17)) - .getMessage()); - assertEquals(closedMessage, - assertThrows(IllegalStateException.class, () -> context.peekVariableValue("banner.border", 17)) - .getMessage()); - assertEquals(closedMessage, - assertThrows(IllegalStateException.class, context::getVariableKeys).getMessage()); + + assertEquals(0, context.peekTreatment("exp_test_ab")); + assertEquals(0, context.getTreatment("exp_test_ab")); + assertArrayEquals(new String[0], context.getExperiments()); + assertEquals(17, context.getVariableValue("banner.border", 17)); + assertEquals(17, context.peekVariableValue("banner.border", 17)); + assertEquals(new HashMap<>(), context.getVariableKeys()); + assertArrayEquals(new String[0], context.getCustomFieldKeys()); + assertNull(context.getCustomFieldValue("exp_test_ab", "key")); + assertNull(context.getCustomFieldValueType("exp_test_ab", "key")); + } + + @Test + void finalizeIsAliasForClose() { + final Context context = createReadyContext(); + assertFalse(context.isFinalized()); + assertFalse(context.isFinalizing()); + + when(eventHandler.publish(any(), any())).thenReturn(CompletableFuture.completedFuture(null)); + + context.finalize(); + + assertTrue(context.isFinalized()); + assertTrue(context.isClosed()); + } + + @Test + void finalizeAsyncIsAliasForCloseAsync() { + final Context context = createReadyContext(); + assertFalse(context.isFinalized()); + + context.track("goal1", mapOf("amount", 125, "hours", 245)); + + final CompletableFuture publishFuture = new CompletableFuture<>(); + when(eventHandler.publish(any(), any())).thenReturn(publishFuture); + + final CompletableFuture finalizeFuture = context.finalizeAsync(); + assertTrue(context.isFinalizing()); + assertFalse(context.isFinalized()); + + publishFuture.complete(null); + finalizeFuture.join(); + + assertTrue(context.isFinalized()); + assertFalse(context.isFinalizing()); } @Test @@ -473,10 +529,10 @@ void startsRefreshTimerWhenReady() { final AtomicReference runnable = new AtomicReference<>(null); when(scheduler.scheduleWithFixedDelay(any(), eq(config.getRefreshInterval()), eq(config.getRefreshInterval()), eq(TimeUnit.MILLISECONDS))) - .thenAnswer(invokation -> { - runnable.set(invokation.getArgument(0)); - return mock(ScheduledFuture.class); - }); + .thenAnswer(invokation -> { + runnable.set(invokation.getArgument(0)); + return mock(ScheduledFuture.class); + }); dataFuture.complete(data); context.waitUntilReady(); @@ -1814,7 +1870,7 @@ void closeStopsRefreshTimer() { final ScheduledFuture refreshTimer = mock(ScheduledFuture.class); when(scheduler.scheduleWithFixedDelay(any(), eq(config.getRefreshInterval()), eq(config.getRefreshInterval()), eq(TimeUnit.MILLISECONDS))) - .thenReturn(refreshTimer); + .thenReturn(refreshTimer); final Context context = createContext(config, dataFutureReady); assertTrue(context.isReady()); @@ -2203,4 +2259,504 @@ void getCustomFieldValue() { assertNull(context.getCustomFieldValue("exp_test_no_custom_fields", "languages")); assertNull(context.getCustomFieldValueType("exp_test_no_custom_fields", "languages")); } + + @Test + void getTreatmentQueuesExposureAfterPeek() { + final Context context = createReadyContext(); + + Arrays.stream(data.experiments).forEach(experiment -> context.peekTreatment(experiment.name)); + context.peekTreatment("not_found"); + + assertEquals(0, context.getPendingCount()); + + Arrays.stream(data.experiments).forEach(experiment -> context.getTreatment(experiment.name)); + context.getTreatment("not_found"); + + assertEquals(1 + data.experiments.length, context.getPendingCount()); + } + + @Test + void getTreatmentQueuesExposureWithBaseVariantOnUnknownExperiment() { + final Context context = createReadyContext(); + + assertEquals(0, context.getTreatment("not_found")); + assertEquals(1, context.getPendingCount()); + + when(eventHandler.publish(any(), any())).thenReturn(CompletableFuture.completedFuture(null)); + + context.publish(); + + final PublishEvent expected = new PublishEvent(); + expected.hashed = true; + expected.publishedAt = clock.millis(); + expected.units = publishUnits; + + expected.exposures = new Exposure[]{ + new Exposure(0, "not_found", null, 0, clock.millis(), false, true, false, false, false, false), + }; + + when(eventHandler.publish(any(), any())).thenReturn(CompletableFuture.completedFuture(null)); + + context.publish(); + + verify(eventHandler, Mockito.timeout(5000).times(1)).publish(any(), any()); + verify(eventHandler, Mockito.timeout(5000).times(1)).publish(context, expected); + } + + @Test + void getTreatmentDoesNotReQueueExposureOnUnknownExperiment() { + final Context context = createReadyContext(); + + assertEquals(0, context.getTreatment("not_found")); + assertEquals(1, context.getPendingCount()); + + assertEquals(0, context.getTreatment("not_found")); + assertEquals(1, context.getPendingCount()); + + when(eventHandler.publish(any(), any())).thenReturn(CompletableFuture.completedFuture(null)); + + context.publish(); + + assertEquals(0, context.getTreatment("not_found")); + assertEquals(0, context.getPendingCount()); + } + + @Test + void getTreatmentQueuesExposureWithCustomAssignmentVariant() { + final Context context = createReadyContext(); + + context.setCustomAssignment("exp_test_ab", 2); + + assertEquals(2, context.getTreatment("exp_test_ab")); + assertEquals(1, context.getPendingCount()); + + when(eventHandler.publish(any(), any())).thenReturn(CompletableFuture.completedFuture(null)); + + context.publish(); + + final PublishEvent expected = new PublishEvent(); + expected.hashed = true; + expected.publishedAt = clock.millis(); + expected.units = publishUnits; + + expected.exposures = new Exposure[]{ + new Exposure(1, "exp_test_ab", "session_id", 2, clock.millis(), true, true, false, false, true, false), + }; + + when(eventHandler.publish(any(), any())).thenReturn(CompletableFuture.completedFuture(null)); + + context.publish(); + + verify(eventHandler, Mockito.timeout(5000).times(1)).publish(any(), any()); + verify(eventHandler, Mockito.timeout(5000).times(1)).publish(context, expected); + } + + @Test + void getVariableValueReturnsDefaultValueWhenUnassigned() { + final Context context = createReadyContext(); + + assertEquals(17, context.getVariableValue("card.width", 17)); + } + + @Test + void getVariableValueReturnsDefaultValueOnUnknownVariable() { + final Context context = createReadyContext(); + + assertEquals("default", context.getVariableValue("unknown_variable", "default")); + assertEquals(0, context.getPendingCount()); + } + + @Test + void getVariableValueReturnsVariableValuesWhenOverridden() { + final Context context = createReadyContext(); + + context.setOverride("exp_test_ab", 0); + + assertEquals(17, context.getVariableValue("banner.border", 17)); + } + + @Test + void getVariableValueQueuesExposureAfterPeekVariableValue() { + final Context context = createReadyContext(); + + context.peekVariableValue("banner.border", 17); + context.peekVariableValue("banner.size", 17); + + assertEquals(0, context.getPendingCount()); + + context.getVariableValue("banner.border", 17); + context.getVariableValue("banner.size", 17); + + assertEquals(1, context.getPendingCount()); + } + + @Test + void getVariableValueQueuesExposureOnce() { + final Context context = createReadyContext(); + + context.getVariableValue("banner.border", 17); + context.getVariableValue("banner.size", 17); + + assertEquals(1, context.getPendingCount()); + + context.getVariableValue("banner.border", 17); + context.getVariableValue("banner.size", 17); + + assertEquals(1, context.getPendingCount()); + } + + @Test + void peekVariableValueReturnsDefaultValueWhenUnassigned() { + final Context context = createReadyContext(); + + assertEquals(17, context.peekVariableValue("card.width", 17)); + } + + @Test + void peekVariableValueReturnsVariableValuesWhenOverridden() { + final Context context = createReadyContext(); + + context.setOverride("exp_test_ab", 0); + + assertEquals(17, context.peekVariableValue("banner.border", 17)); + } + + @Test + void peekVariableValueReturnsDefaultValueOnUnknownOverrideVariant() { + final Context context = createReadyContext(); + + context.setOverride("exp_test_ab", 15); + + assertEquals(17, context.peekVariableValue("banner.border", 17)); + } + + @Test + void refreshKeepsOverrides() { + final Context context = createReadyContext(); + + context.setOverride("exp_test_ab", 5); + assertEquals(5, context.getOverride("exp_test_ab")); + + when(dataProvider.getContextData()).thenReturn(refreshDataFutureReady); + + context.refresh(); + + assertEquals(5, context.getOverride("exp_test_ab")); + assertEquals(5, context.getTreatment("exp_test_ab")); + } + + @Test + void refreshKeepsCustomAssignments() { + final Context context = createReadyContext(); + + context.setCustomAssignment("exp_test_ab", 2); + assertEquals(2, context.getCustomAssignment("exp_test_ab")); + + when(dataProvider.getContextData()).thenReturn(refreshDataFutureReady); + + context.refresh(); + + assertEquals(2, context.getCustomAssignment("exp_test_ab")); + assertEquals(2, context.getTreatment("exp_test_ab")); + } + + @Test + void refreshDoesNotCallPublishWhenFailed() { + final Context context = createContext(dataFutureFailed); + assertTrue(context.isReady()); + assertTrue(context.isFailed()); + + when(dataProvider.getContextData()).thenReturn(refreshDataFutureReady); + + context.refresh(); + + verify(dataProvider, Mockito.timeout(5000).times(1)).getContextData(); + } + + @Test + void publishIncludesExposureData() { + final Context context = createReadyContext(); + + context.getTreatment("exp_test_ab"); + + assertEquals(1, context.getPendingCount()); + + final PublishEvent expected = new PublishEvent(); + expected.hashed = true; + expected.publishedAt = clock.millis(); + expected.units = publishUnits; + expected.exposures = new Exposure[]{ + new Exposure(1, "exp_test_ab", "session_id", 1, clock.millis(), true, true, false, false, false, false), + }; + + when(eventHandler.publish(context, expected)).thenReturn(CompletableFuture.completedFuture(null)); + + context.publish(); + + verify(eventHandler, Mockito.timeout(5000).times(1)).publish(any(), any()); + verify(eventHandler, Mockito.timeout(5000).times(1)).publish(context, expected); + } + + @Test + void publishIncludesGoalData() { + final Context context = createReadyContext(); + + context.track("goal1", mapOf("amount", 125, "hours", 245)); + + assertEquals(1, context.getPendingCount()); + + final PublishEvent expected = new PublishEvent(); + expected.hashed = true; + expected.publishedAt = clock.millis(); + expected.units = publishUnits; + expected.goals = new GoalAchievement[]{ + new GoalAchievement("goal1", clock.millis(), new TreeMap<>(mapOf("amount", 125, "hours", 245))), + }; + + when(eventHandler.publish(context, expected)).thenReturn(CompletableFuture.completedFuture(null)); + + context.publish(); + + verify(eventHandler, Mockito.timeout(5000).times(1)).publish(any(), any()); + verify(eventHandler, Mockito.timeout(5000).times(1)).publish(context, expected); + } + + @Test + void publishIncludesAttributeData() { + final ContextConfig config = ContextConfig.create() + .setUnits(units) + .setAttributes(mapOf("attr1", "value1")); + + final Context context = createContext(config, dataFutureReady); + + context.track("goal1", null); + + final PublishEvent expected = new PublishEvent(); + expected.hashed = true; + expected.publishedAt = clock.millis(); + expected.units = publishUnits; + expected.goals = new GoalAchievement[]{ + new GoalAchievement("goal1", clock.millis(), null), + }; + expected.attributes = new Attribute[]{ + new Attribute("attr1", "value1", clock.millis()), + }; + + when(eventHandler.publish(context, expected)).thenReturn(CompletableFuture.completedFuture(null)); + + context.publish(); + + verify(eventHandler, Mockito.timeout(5000).times(1)).publish(any(), any()); + verify(eventHandler, Mockito.timeout(5000).times(1)).publish(context, expected); + } + + @Test + void publishClearsQueueOnSuccess() { + final Context context = createReadyContext(); + + context.track("goal1", mapOf("amount", 125)); + assertEquals(1, context.getPendingCount()); + + when(eventHandler.publish(any(), any())).thenReturn(CompletableFuture.completedFuture(null)); + + context.publish(); + + assertEquals(0, context.getPendingCount()); + } + + @Test + void publishPropagatesClientErrorOnFailure() { + final Context context = createReadyContext(); + + context.track("goal1", mapOf("amount", 125)); + assertEquals(1, context.getPendingCount()); + + final Exception failure = new Exception("publish error"); + when(eventHandler.publish(any(), any())).thenReturn(failedFuture(failure)); + + final CompletionException actual = assertThrows(CompletionException.class, context::publish); + assertSame(failure, actual.getCause()); + } + + @Test + void closeDoesNotCallEventHandlerWhenQueueIsEmpty() { + final Context context = createReadyContext(); + assertEquals(0, context.getPendingCount()); + + context.close(); + + assertTrue(context.isClosed()); + verify(eventHandler, Mockito.timeout(5000).times(0)).publish(any(), any()); + } + + @Test + void closeCallsEventHandlerWithPendingData() { + final Context context = createReadyContext(); + + context.track("goal1", mapOf("amount", 125)); + + when(eventHandler.publish(any(), any())).thenReturn(CompletableFuture.completedFuture(null)); + + context.close(); + + assertTrue(context.isClosed()); + verify(eventHandler, Mockito.timeout(5000).times(1)).publish(any(), any()); + } + + @Test + void closeDoesNotCallEventHandlerWhenFailed() { + final Context context = createContext(dataFutureFailed); + assertTrue(context.isReady()); + assertTrue(context.isFailed()); + + context.getTreatment("exp_test_abc"); + context.track("goal1", mapOf("amount", 125)); + + context.close(); + + assertTrue(context.isClosed()); + verify(eventHandler, Mockito.timeout(5000).times(0)).publish(any(), any()); + } + + @Test + void closeAsyncReturnsSameFutureWhenCalledTwice() { + final Context context = createReadyContext(); + + context.track("goal1", mapOf("amount", 125)); + + final CompletableFuture publishFuture = new CompletableFuture<>(); + when(eventHandler.publish(any(), any())).thenReturn(publishFuture); + + final CompletableFuture closeFuture1 = context.closeAsync(); + final CompletableFuture closeFuture2 = context.closeAsync(); + + assertSame(closeFuture1, closeFuture2); + + publishFuture.complete(null); + closeFuture1.join(); + + assertTrue(context.isClosed()); + } + + @Test + void closeAsyncReturnsCompletedFutureWhenAlreadyClosed() { + final Context context = createReadyContext(); + + context.close(); + assertTrue(context.isClosed()); + + final CompletableFuture closeFuture = context.closeAsync(); + assertTrue(closeFuture.isDone()); + } + + @Test + void trackQueuesGoalWithProperties() { + final Context context = createReadyContext(); + + final Map properties = mapOf("amount", 125, "hours", 245); + context.track("goal1", properties); + + assertEquals(1, context.getPendingCount()); + } + + @Test + void trackQueuesGoalWithNullProperties() { + final Context context = createReadyContext(); + + context.track("goal1", null); + + assertEquals(1, context.getPendingCount()); + } + + @Test + void getVariableValueConflictingKeyOverlappingAudiences() { + for (final Experiment experiment : data.experiments) { + switch (experiment.name) { + case "exp_test_ab": + assert (expectedVariants.get(experiment.name) != 0); + experiment.audienceStrict = true; + experiment.audience = "{\"filter\":[{\"gte\":[{\"var\":\"age\"},{\"value\":20}]}]}"; + experiment.variants[expectedVariants.get(experiment.name)].config = "{\"icon\":\"arrow\"}"; + break; + case "exp_test_abc": + assert (expectedVariants.get(experiment.name) != 0); + experiment.audienceStrict = true; + experiment.audience = "{\"filter\":[{\"gte\":[{\"var\":\"age\"},{\"value\":20}]}]}"; + experiment.variants[expectedVariants.get(experiment.name)].config = "{\"icon\":\"circle\"}"; + break; + default: + break; + } + } + + final Context context = createReadyContext(data); + context.setAttribute("age", 25); + assertEquals("arrow", context.getVariableValue("icon", "square")); + assertEquals(1, context.getPendingCount()); + } + + @Test + void publishKeepsEventsPendingOnFailure() { + final Context context = createReadyContext(); + + context.track("goal1", mapOf("amount", 125)); + assertEquals(1, context.getPendingCount()); + + final Exception failure = new Exception("PUBLISH_FAILED"); + when(eventHandler.publish(any(), any())).thenReturn(failedFuture(failure)); + + assertThrows(CompletionException.class, context::publish); + + assertEquals(1, context.getPendingCount()); + } + + @Test + void setOverrideSucceedsAfterClose() { + final Context context = createReadyContext(); + + context.close(); + assertTrue(context.isClosed()); + + context.setOverride("exp_test_ab", 2); + assertEquals(2, context.getOverride("exp_test_ab")); + } + + @Test + void setOverridesSucceedsAfterClose() { + final Context context = createReadyContext(); + + context.close(); + assertTrue(context.isClosed()); + + context.setOverrides(mapOf("exp_test_ab", 2, "exp_test_abc", 1)); + assertEquals(2, context.getOverride("exp_test_ab")); + assertEquals(1, context.getOverride("exp_test_abc")); + } + + @Test + void peekVariableValueConflictingKeyOverlappingAudiences() { + for (final Experiment experiment : data.experiments) { + switch (experiment.name) { + case "exp_test_ab": + assert (expectedVariants.get(experiment.name) != 0); + experiment.audienceStrict = true; + experiment.audience = "{\"filter\":[{\"gte\":[{\"var\":\"age\"},{\"value\":20}]}]}"; + experiment.variants[expectedVariants.get(experiment.name)].config = "{\"icon\":\"arrow\"}"; + break; + case "exp_test_abc": + assert (expectedVariants.get(experiment.name) != 0); + experiment.audienceStrict = true; + experiment.audience = "{\"filter\":[{\"gte\":[{\"var\":\"age\"},{\"value\":20}]}]}"; + experiment.variants[expectedVariants.get(experiment.name)].config = "{\"icon\":\"circle\"}"; + break; + default: + break; + } + } + + final Context context = createReadyContext(data); + context.setAttribute("age", 25); + assertEquals("arrow", context.peekVariableValue("icon", "square")); + assertEquals(0, context.getPendingCount()); + } } diff --git a/core-api/src/test/java/com/absmartly/sdk/DefaultContextDataDeserializerTest.java b/core-api/src/test/java/com/absmartly/sdk/DefaultContextDataDeserializerTest.java index d1966d8..ea4141e 100644 --- a/core-api/src/test/java/com/absmartly/sdk/DefaultContextDataDeserializerTest.java +++ b/core-api/src/test/java/com/absmartly/sdk/DefaultContextDataDeserializerTest.java @@ -123,4 +123,115 @@ void deserializeDoesNotThrow() { assertNull(data); }); } + + @Test + void testMalformedJsonResponse() { + final ContextDataDeserializer deser = new DefaultContextDataDeserializer(); + + final byte[] malformedJson = "{\"experiments\": [".getBytes(); + final ContextData result = deser.deserialize(malformedJson, 0, malformedJson.length); + assertNull(result); + + final byte[] invalidJson = "not a json at all".getBytes(); + final ContextData result2 = deser.deserialize(invalidJson, 0, invalidJson.length); + assertNull(result2); + + final byte[] emptyBraces = "{}".getBytes(); + final ContextData result3 = deser.deserialize(emptyBraces, 0, emptyBraces.length); + assertNotNull(result3); + + final byte[] emptyArray = "[]".getBytes(); + final ContextData result4 = deser.deserialize(emptyArray, 0, emptyArray.length); + assertNull(result4); + } + + @Test + void testEmptyExperimentsArray() { + final ContextDataDeserializer deser = new DefaultContextDataDeserializer(); + + final byte[] emptyExperiments = "{\"experiments\": []}".getBytes(); + final ContextData result = deser.deserialize(emptyExperiments, 0, emptyExperiments.length); + assertNotNull(result); + assertNotNull(result.experiments); + assertEquals(0, result.experiments.length); + } + + @Test + void testPartialResponseHandling() { + final ContextDataDeserializer deser = new DefaultContextDataDeserializer(); + + final byte[] partialExperiment = "{\"experiments\": [{\"id\": 1, \"name\": \"test\"}]}".getBytes(); + final ContextData result = deser.deserialize(partialExperiment, 0, partialExperiment.length); + assertNotNull(result); + assertNotNull(result.experiments); + assertEquals(1, result.experiments.length); + assertEquals(1, result.experiments[0].id); + assertEquals("test", result.experiments[0].name); + assertNull(result.experiments[0].unitType); + assertNull(result.experiments[0].variants); + assertNull(result.experiments[0].split); + + final byte[] missingVariants = ("{\"experiments\": [{" + + "\"id\": 1, " + + "\"name\": \"exp_test\", " + + "\"unitType\": \"session_id\", " + + "\"iteration\": 1, " + + "\"seedHi\": 100, " + + "\"seedLo\": 200" + + "}]}").getBytes(); + final ContextData result2 = deser.deserialize(missingVariants, 0, missingVariants.length); + assertNotNull(result2); + assertNotNull(result2.experiments); + assertEquals(1, result2.experiments.length); + assertNull(result2.experiments[0].variants); + } + + @Test + void testNullFieldsInExperiment() { + final ContextDataDeserializer deser = new DefaultContextDataDeserializer(); + + final byte[] withNulls = ("{\"experiments\": [{" + + "\"id\": 1, " + + "\"name\": \"exp_test\", " + + "\"unitType\": \"session_id\", " + + "\"iteration\": 1, " + + "\"seedHi\": 100, " + + "\"seedLo\": 200, " + + "\"split\": null, " + + "\"trafficSplit\": null, " + + "\"variants\": null, " + + "\"audience\": null" + + "}]}").getBytes(); + final ContextData result = deser.deserialize(withNulls, 0, withNulls.length); + assertNotNull(result); + assertNotNull(result.experiments); + assertEquals(1, result.experiments.length); + assertNull(result.experiments[0].split); + assertNull(result.experiments[0].trafficSplit); + assertNull(result.experiments[0].variants); + assertNull(result.experiments[0].audience); + } + + @Test + void testEmptyByteArray() { + final ContextDataDeserializer deser = new DefaultContextDataDeserializer(); + + final byte[] empty = new byte[0]; + final ContextData result = deser.deserialize(empty, 0, 0); + assertNull(result); + } + + @Test + void testOffsetAndLength() { + final byte[] bytes = getResourceBytes("context.json"); + final ContextDataDeserializer deser = new DefaultContextDataDeserializer(); + + final ContextData partialResult = deser.deserialize(bytes, 0, 10); + assertNull(partialResult); + + final ContextData fullResult = deser.deserialize(bytes, 0, bytes.length); + assertNotNull(fullResult); + assertNotNull(fullResult.experiments); + assertTrue(fullResult.experiments.length > 0); + } } diff --git a/core-api/src/test/java/com/absmartly/sdk/DefaultContextDataProviderTest.java b/core-api/src/test/java/com/absmartly/sdk/DefaultContextDataProviderTest.java index 487acb8..9ff8365 100644 --- a/core-api/src/test/java/com/absmartly/sdk/DefaultContextDataProviderTest.java +++ b/core-api/src/test/java/com/absmartly/sdk/DefaultContextDataProviderTest.java @@ -43,4 +43,60 @@ void getContextDataExceptionally() { verify(client, Mockito.timeout(5000).times(1)).getContextData(); } + + @Test + void getContextDataWithEmptyExperiments() throws ExecutionException, InterruptedException { + final Client client = mock(Client.class); + final ContextDataProvider provider = new DefaultContextDataProvider(client); + + final ContextData emptyData = new ContextData(); + emptyData.experiments = new com.absmartly.sdk.json.Experiment[0]; + when(client.getContextData()).thenReturn(CompletableFuture.completedFuture(emptyData)); + + final CompletableFuture dataFuture = provider.getContextData(); + final ContextData actual = dataFuture.get(); + + assertNotNull(actual); + assertNotNull(actual.experiments); + assertEquals(0, actual.experiments.length); + } + + @Test + void getContextDataMultipleCalls() throws ExecutionException, InterruptedException { + final Client client = mock(Client.class); + final ContextDataProvider provider = new DefaultContextDataProvider(client); + + final ContextData firstData = new ContextData(); + final ContextData secondData = new ContextData(); + when(client.getContextData()) + .thenReturn(CompletableFuture.completedFuture(firstData)) + .thenReturn(CompletableFuture.completedFuture(secondData)); + + final CompletableFuture firstFuture = provider.getContextData(); + final ContextData firstActual = firstFuture.get(); + assertSame(firstData, firstActual); + + final CompletableFuture secondFuture = provider.getContextData(); + final ContextData secondActual = secondFuture.get(); + assertSame(secondData, secondActual); + + verify(client, Mockito.timeout(5000).times(2)).getContextData(); + } + + @Test + void getContextDataWithTimeoutException() { + final Client client = mock(Client.class); + final ContextDataProvider provider = new DefaultContextDataProvider(client); + + final java.util.concurrent.TimeoutException timeoutException = new java.util.concurrent.TimeoutException( + "Request timed out"); + final CompletableFuture failedFuture = failedFuture(timeoutException); + when(client.getContextData()).thenReturn(failedFuture); + + final CompletableFuture dataFuture = provider.getContextData(); + final CompletionException actual = assertThrows(CompletionException.class, dataFuture::join); + assertTrue(actual.getCause() instanceof java.util.concurrent.TimeoutException); + + verify(client, Mockito.timeout(5000).times(1)).getContextData(); + } } diff --git a/core-api/src/test/java/com/absmartly/sdk/DefaultHTTPClientConfigTest.java b/core-api/src/test/java/com/absmartly/sdk/DefaultHTTPClientConfigTest.java index bb42d66..4af971b 100644 --- a/core-api/src/test/java/com/absmartly/sdk/DefaultHTTPClientConfigTest.java +++ b/core-api/src/test/java/com/absmartly/sdk/DefaultHTTPClientConfigTest.java @@ -59,4 +59,57 @@ void setHttpVersionPolicy() { .setHTTPVersionPolicy(HTTPVersionPolicy.FORCE_HTTP_1); assertEquals(HTTPVersionPolicy.FORCE_HTTP_1, config.getHTTPVersionPolicy()); } + + @Test + void testNegativeConnectTimeout() { + final DefaultHTTPClientConfig config = DefaultHTTPClientConfig.create() + .setConnectTimeout(-1); + assertEquals(-1, config.getConnectTimeout()); + } + + @Test + void testNegativeConnectionKeepAlive() { + final DefaultHTTPClientConfig config = DefaultHTTPClientConfig.create() + .setConnectionKeepAlive(-1); + assertEquals(-1, config.getConnectionKeepAlive()); + } + + @Test + void testNegativeConnectionRequestTimeout() { + final DefaultHTTPClientConfig config = DefaultHTTPClientConfig.create() + .setConnectionRequestTimeout(-1); + assertEquals(-1, config.getConnectionRequestTimeout()); + } + + @Test + void testNegativeRetryInterval() { + final DefaultHTTPClientConfig config = DefaultHTTPClientConfig.create() + .setRetryInterval(-1); + assertEquals(-1, config.getRetryInterval()); + } + + @Test + void testZeroMaxRetries() { + final DefaultHTTPClientConfig config = DefaultHTTPClientConfig.create() + .setMaxRetries(0); + assertEquals(0, config.getMaxRetries()); + } + + @Test + void testNegativeMaxRetries() { + final DefaultHTTPClientConfig config = DefaultHTTPClientConfig.create() + .setMaxRetries(-1); + assertEquals(-1, config.getMaxRetries()); + } + + @Test + void testDefaultValues() { + final DefaultHTTPClientConfig config = DefaultHTTPClientConfig.create(); + assertEquals(3000, config.getConnectTimeout()); + assertEquals(30000, config.getConnectionKeepAlive()); + assertEquals(1000, config.getConnectionRequestTimeout()); + assertEquals(5, config.getMaxRetries()); + assertEquals(333, config.getRetryInterval()); + assertEquals(HTTPVersionPolicy.NEGOTIATE, config.getHTTPVersionPolicy()); + } } diff --git a/core-api/src/test/java/com/absmartly/sdk/DefaultHTTPClientTest.java b/core-api/src/test/java/com/absmartly/sdk/DefaultHTTPClientTest.java index 0a3a4fd..150110b 100644 --- a/core-api/src/test/java/com/absmartly/sdk/DefaultHTTPClientTest.java +++ b/core-api/src/test/java/com/absmartly/sdk/DefaultHTTPClientTest.java @@ -4,12 +4,15 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.*; +import java.net.SocketTimeoutException; import java.util.HashMap; import java.util.Map; import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; import java8.util.concurrent.CompletableFuture; import java8.util.concurrent.CompletionException; +import org.apache.hc.client5.http.ConnectTimeoutException; import org.apache.hc.client5.http.async.methods.SimpleHttpRequest; import org.apache.hc.client5.http.async.methods.SimpleHttpResponse; import org.apache.hc.client5.http.impl.async.CloseableHttpAsyncClient; @@ -22,6 +25,7 @@ import org.apache.hc.core5.util.TimeValue; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; import org.mockito.MockedStatic; import org.mockito.Mockito; @@ -286,4 +290,130 @@ void post() throws ExecutionException, InterruptedException { verify(asyncHTTPClient, Mockito.timeout(5000).times(1)).execute(any(), any()); } } + + @Test + @Timeout(value = 5, unit = TimeUnit.SECONDS) + void testConnectionTimeout() { + try (final MockedStatic builderStatic = Mockito + .mockStatic(HttpAsyncClientBuilder.class)) { + builderStatic.when(HttpAsyncClientBuilder::create).thenReturn(asyncHTTPClientBuilder); + + final DefaultHTTPClient httpClient = DefaultHTTPClient.create( + DefaultHTTPClientConfig.create().setConnectTimeout(100)); + + final ConnectTimeoutException timeoutException = new ConnectTimeoutException("Connection timed out"); + when(asyncHTTPClient.execute(any(), any())).thenAnswer(invocation -> { + final FutureCallback callback = invocation.getArgument(1); + callback.failed(timeoutException); + return null; + }); + + final CompletableFuture responseFuture = httpClient + .get("https://api.absmartly.com/v1/context", null, null); + + final CompletionException thrown = assertThrows(CompletionException.class, responseFuture::join); + assertSame(timeoutException, thrown.getCause()); + assertTrue(thrown.getCause() instanceof ConnectTimeoutException); + } + } + + @Test + @Timeout(value = 5, unit = TimeUnit.SECONDS) + void testReadTimeout() { + try (final MockedStatic builderStatic = Mockito + .mockStatic(HttpAsyncClientBuilder.class)) { + builderStatic.when(HttpAsyncClientBuilder::create).thenReturn(asyncHTTPClientBuilder); + + final DefaultHTTPClient httpClient = DefaultHTTPClient.create(DefaultHTTPClientConfig.create()); + + final SocketTimeoutException readTimeoutException = new SocketTimeoutException("Read timed out"); + when(asyncHTTPClient.execute(any(), any())).thenAnswer(invocation -> { + final FutureCallback callback = invocation.getArgument(1); + callback.failed(readTimeoutException); + return null; + }); + + final CompletableFuture responseFuture = httpClient + .get("https://api.absmartly.com/v1/context", null, null); + + final CompletionException thrown = assertThrows(CompletionException.class, responseFuture::join); + assertSame(readTimeoutException, thrown.getCause()); + assertTrue(thrown.getCause() instanceof SocketTimeoutException); + } + } + + @Test + void testRateLimiting429Response() throws ExecutionException, InterruptedException { + try (final MockedStatic builderStatic = Mockito + .mockStatic(HttpAsyncClientBuilder.class)) { + builderStatic.when(HttpAsyncClientBuilder::create).thenReturn(asyncHTTPClientBuilder); + + final DefaultHTTPClient httpClient = DefaultHTTPClient.create(DefaultHTTPClientConfig.create()); + + when(asyncHTTPClient.execute(any(), any())).thenAnswer(invocation -> { + final FutureCallback callback = invocation.getArgument(1); + callback.completed(SimpleHttpResponse.create(429, "Too Many Requests".getBytes(), + ContentType.TEXT_PLAIN)); + return null; + }); + + final CompletableFuture responseFuture = httpClient + .get("https://api.absmartly.com/v1/context", null, null); + final HTTPClient.Response response = responseFuture.get(); + + assertEquals(429, response.getStatusCode()); + assertEquals("text/plain", response.getContentType()); + } + } + + @Test + void testRetryOnTransientError503Response() throws ExecutionException, InterruptedException { + try (final MockedStatic builderStatic = Mockito + .mockStatic(HttpAsyncClientBuilder.class)) { + builderStatic.when(HttpAsyncClientBuilder::create).thenReturn(asyncHTTPClientBuilder); + + final DefaultHTTPClient httpClient = DefaultHTTPClient.create( + DefaultHTTPClientConfig.create().setMaxRetries(3).setRetryInterval(100)); + + when(asyncHTTPClient.execute(any(), any())).thenAnswer(invocation -> { + final FutureCallback callback = invocation.getArgument(1); + callback.completed(SimpleHttpResponse.create(503, "Service Unavailable".getBytes(), + ContentType.TEXT_PLAIN)); + return null; + }); + + final CompletableFuture responseFuture = httpClient + .get("https://api.absmartly.com/v1/context", null, null); + final HTTPClient.Response response = responseFuture.get(); + + assertEquals(503, response.getStatusCode()); + + verify(asyncHTTPClient, Mockito.timeout(5000).times(1)).execute(any(), any()); + } + } + + @Test + void testSSLCertificateValidation() { + try (final MockedStatic builderStatic = Mockito + .mockStatic(HttpAsyncClientBuilder.class)) { + builderStatic.when(HttpAsyncClientBuilder::create).thenReturn(asyncHTTPClientBuilder); + + final DefaultHTTPClient httpClient = DefaultHTTPClient.create(DefaultHTTPClientConfig.create()); + + final javax.net.ssl.SSLHandshakeException sslException = new javax.net.ssl.SSLHandshakeException( + "Certificate validation failed"); + when(asyncHTTPClient.execute(any(), any())).thenAnswer(invocation -> { + final FutureCallback callback = invocation.getArgument(1); + callback.failed(sslException); + return null; + }); + + final CompletableFuture responseFuture = httpClient + .get("https://api.absmartly.com/v1/context", null, null); + + final CompletionException thrown = assertThrows(CompletionException.class, responseFuture::join); + assertSame(sslException, thrown.getCause()); + assertTrue(thrown.getCause() instanceof javax.net.ssl.SSLHandshakeException); + } + } } diff --git a/core-api/src/test/java/com/absmartly/sdk/internal/ConcurrencyTest.java b/core-api/src/test/java/com/absmartly/sdk/internal/ConcurrencyTest.java index ba1d46c..5b246c8 100644 --- a/core-api/src/test/java/com/absmartly/sdk/internal/ConcurrencyTest.java +++ b/core-api/src/test/java/com/absmartly/sdk/internal/ConcurrencyTest.java @@ -1,19 +1,22 @@ package com.absmartly.sdk.internal; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.*; -import java.util.Map; +import java.util.*; +import java.util.concurrent.*; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.locks.ReentrantReadWriteLock; import java8.util.function.Function; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; import org.mockito.Mockito; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; import com.absmartly.sdk.TestUtils; + import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; @SuppressFBWarnings(value = "RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT") @@ -142,4 +145,200 @@ void putRW() { verify(lock, Mockito.timeout(5000).times(1)).lock(); verify(lock, Mockito.timeout(5000).times(1)).unlock(); } + + @Test + @Timeout(value = 10, unit = TimeUnit.SECONDS) + void testConcurrentGetOperations() throws InterruptedException { + final ReentrantReadWriteLock rwlock = new ReentrantReadWriteLock(); + final Map map = new ConcurrentHashMap<>(); + map.put("key1", 100); + map.put("key2", 200); + map.put("key3", 300); + + final int threadCount = 10; + final int operationsPerThread = 1000; + final ExecutorService executor = Executors.newFixedThreadPool(threadCount); + final CountDownLatch startLatch = new CountDownLatch(1); + final CountDownLatch doneLatch = new CountDownLatch(threadCount); + final AtomicInteger successCount = new AtomicInteger(0); + + for (int i = 0; i < threadCount; i++) { + executor.submit(() -> { + try { + startLatch.await(); + for (int j = 0; j < operationsPerThread; j++) { + final Integer value = Concurrency.getRW(rwlock, map, "key" + ((j % 3) + 1)); + if (value != null) { + successCount.incrementAndGet(); + } + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } finally { + doneLatch.countDown(); + } + }); + } + + startLatch.countDown(); + doneLatch.await(); + executor.shutdown(); + + assertEquals(threadCount * operationsPerThread, successCount.get()); + } + + @Test + @Timeout(value = 10, unit = TimeUnit.SECONDS) + void testConcurrentPutOperations() throws InterruptedException { + final ReentrantReadWriteLock rwlock = new ReentrantReadWriteLock(); + final Map map = new ConcurrentHashMap<>(); + + final int threadCount = 10; + final int operationsPerThread = 100; + final ExecutorService executor = Executors.newFixedThreadPool(threadCount); + final CountDownLatch startLatch = new CountDownLatch(1); + final CountDownLatch doneLatch = new CountDownLatch(threadCount); + + for (int i = 0; i < threadCount; i++) { + final int threadId = i; + executor.submit(() -> { + try { + startLatch.await(); + for (int j = 0; j < operationsPerThread; j++) { + final String key = "thread" + threadId + "_key" + j; + Concurrency.putRW(rwlock, map, key, threadId * 1000 + j); + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } finally { + doneLatch.countDown(); + } + }); + } + + startLatch.countDown(); + doneLatch.await(); + executor.shutdown(); + + assertEquals(threadCount * operationsPerThread, map.size()); + } + + @Test + @Timeout(value = 10, unit = TimeUnit.SECONDS) + void testConcurrentComputeIfAbsent() throws InterruptedException { + final ReentrantReadWriteLock rwlock = new ReentrantReadWriteLock(); + final Map map = new ConcurrentHashMap<>(); + final AtomicInteger computeCount = new AtomicInteger(0); + + final int threadCount = 10; + final ExecutorService executor = Executors.newFixedThreadPool(threadCount); + final CountDownLatch startLatch = new CountDownLatch(1); + final CountDownLatch doneLatch = new CountDownLatch(threadCount); + + final Function computer = key -> { + computeCount.incrementAndGet(); + return 42; + }; + + for (int i = 0; i < threadCount; i++) { + executor.submit(() -> { + try { + startLatch.await(); + final Integer result = Concurrency.computeIfAbsentRW(rwlock, map, "shared_key", computer); + assertEquals(42, result); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } finally { + doneLatch.countDown(); + } + }); + } + + startLatch.countDown(); + doneLatch.await(); + executor.shutdown(); + + assertEquals(1, map.size()); + assertEquals(1, computeCount.get()); + } + + @Test + @Timeout(value = 10, unit = TimeUnit.SECONDS) + void testConcurrentReadWriteMixed() throws InterruptedException { + final ReentrantReadWriteLock rwlock = new ReentrantReadWriteLock(); + final Map map = new ConcurrentHashMap<>(); + map.put("counter", 0); + + final int threadCount = 20; + final int operationsPerThread = 100; + final ExecutorService executor = Executors.newFixedThreadPool(threadCount); + final CountDownLatch startLatch = new CountDownLatch(1); + final CountDownLatch doneLatch = new CountDownLatch(threadCount); + final AtomicInteger readCount = new AtomicInteger(0); + final AtomicInteger writeCount = new AtomicInteger(0); + + for (int i = 0; i < threadCount; i++) { + final boolean isWriter = i % 2 == 0; + executor.submit(() -> { + try { + startLatch.await(); + for (int j = 0; j < operationsPerThread; j++) { + if (isWriter) { + Concurrency.putRW(rwlock, map, "key" + j, j); + writeCount.incrementAndGet(); + } else { + Concurrency.getRW(rwlock, map, "key" + (j % 50)); + readCount.incrementAndGet(); + } + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } finally { + doneLatch.countDown(); + } + }); + } + + startLatch.countDown(); + doneLatch.await(); + executor.shutdown(); + + assertEquals((threadCount / 2) * operationsPerThread, readCount.get()); + assertEquals((threadCount / 2) * operationsPerThread, writeCount.get()); + } + + @Test + @Timeout(value = 10, unit = TimeUnit.SECONDS) + void testThreadSafeListOperations() throws InterruptedException { + final ReentrantReadWriteLock rwlock = new ReentrantReadWriteLock(); + final List list = Collections.synchronizedList(new ArrayList<>()); + + final int threadCount = 10; + final int operationsPerThread = 100; + final ExecutorService executor = Executors.newFixedThreadPool(threadCount); + final CountDownLatch startLatch = new CountDownLatch(1); + final CountDownLatch doneLatch = new CountDownLatch(threadCount); + + for (int i = 0; i < threadCount; i++) { + final int threadId = i; + executor.submit(() -> { + try { + startLatch.await(); + for (int j = 0; j < operationsPerThread; j++) { + Concurrency.addRW(rwlock, list, threadId * 1000 + j); + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } finally { + doneLatch.countDown(); + } + }); + } + + startLatch.countDown(); + doneLatch.await(); + executor.shutdown(); + + assertEquals(threadCount * operationsPerThread, list.size()); + } } diff --git a/core-api/src/test/java/com/absmartly/sdk/jsonexpr/ExprEvaluatorTest.java b/core-api/src/test/java/com/absmartly/sdk/jsonexpr/ExprEvaluatorTest.java index bcd27be..6e1e9c2 100644 --- a/core-api/src/test/java/com/absmartly/sdk/jsonexpr/ExprEvaluatorTest.java +++ b/core-api/src/test/java/com/absmartly/sdk/jsonexpr/ExprEvaluatorTest.java @@ -333,4 +333,124 @@ void testCompareStrings() { assertEquals(8, evaluator.compare("9", "100")); assertEquals(-8, evaluator.compare("100", "9")); } + + @Test + void testNumberConvertNaN() { + final ExprEvaluator evaluator = new ExprEvaluator(EMPTY_MAP, EMPTY_MAP); + + final Double nanResult = evaluator.numberConvert(Double.NaN); + assertNotNull(nanResult); + assertTrue(Double.isNaN(nanResult)); + + final Double nanFromString = evaluator.numberConvert("NaN"); + assertNotNull(nanFromString); + assertTrue(Double.isNaN(nanFromString)); + } + + @Test + void testNumberConvertInfinity() { + final ExprEvaluator evaluator = new ExprEvaluator(EMPTY_MAP, EMPTY_MAP); + + final Double posInfResult = evaluator.numberConvert(Double.POSITIVE_INFINITY); + assertNotNull(posInfResult); + assertTrue(Double.isInfinite(posInfResult)); + assertTrue(posInfResult > 0); + + final Double negInfResult = evaluator.numberConvert(Double.NEGATIVE_INFINITY); + assertNotNull(negInfResult); + assertTrue(Double.isInfinite(negInfResult)); + assertTrue(negInfResult < 0); + + final Double infFromString = evaluator.numberConvert("Infinity"); + assertNotNull(infFromString); + assertTrue(Double.isInfinite(infFromString)); + + final Double negInfFromString = evaluator.numberConvert("-Infinity"); + assertNotNull(negInfFromString); + assertTrue(Double.isInfinite(negInfFromString)); + assertTrue(negInfFromString < 0); + } + + @Test + void testLargeNumberPrecision() { + final ExprEvaluator evaluator = new ExprEvaluator(EMPTY_MAP, EMPTY_MAP); + + assertEquals(Double.MAX_VALUE, evaluator.numberConvert(Double.MAX_VALUE)); + assertEquals(Double.MIN_VALUE, evaluator.numberConvert(Double.MIN_VALUE)); + assertEquals(-Double.MAX_VALUE, evaluator.numberConvert(-Double.MAX_VALUE)); + + assertEquals(Long.MAX_VALUE, (long) evaluator.numberConvert(Long.MAX_VALUE).doubleValue()); + + final String largeNumberString = "12345678901234567890"; + final Double largeNumber = evaluator.numberConvert(largeNumberString); + assertNotNull(largeNumber); + assertEquals(1.2345678901234568E19, largeNumber, 1e4); + + assertEquals(1E308, evaluator.numberConvert(1E308)); + assertEquals(-1E308, evaluator.numberConvert(-1E308)); + } + + @Test + void testUnicodeStringComparison() { + final ExprEvaluator evaluator = new ExprEvaluator(EMPTY_MAP, EMPTY_MAP); + + assertEquals(0, evaluator.compare("\u00E9", "\u00E9")); + assertEquals(0, evaluator.compare("\u4E2D\u6587", "\u4E2D\u6587")); + assertEquals(0, evaluator.compare("\uD83D\uDE00", "\uD83D\uDE00")); + + assertTrue(evaluator.compare("a", "\u00E1") < 0); + assertTrue(evaluator.compare("\u00E1", "a") > 0); + + assertTrue(evaluator.compare("\u4E00", "\u4E01") < 0); + assertTrue(evaluator.compare("\u4E01", "\u4E00") > 0); + + assertEquals(0, evaluator.compare("", "")); + assertTrue(evaluator.compare("", "\u4E2D") < 0); + assertTrue(evaluator.compare("\u4E2D", "") > 0); + + assertEquals(0, evaluator.compare("\u0041\u0042\u0043", "ABC")); + } + + @Test + void testStringConvertSpecialNumbers() { + final ExprEvaluator evaluator = new ExprEvaluator(EMPTY_MAP, EMPTY_MAP); + + final String nanString = evaluator.stringConvert(Double.NaN); + assertNotNull(nanString); + assertEquals("NaN", nanString); + + final String posInfString = evaluator.stringConvert(Double.POSITIVE_INFINITY); + assertNotNull(posInfString); + assertEquals("\u221E", posInfString); + + final String negInfString = evaluator.stringConvert(Double.NEGATIVE_INFINITY); + assertNotNull(negInfString); + assertEquals("-\u221E", negInfString); + } + + @Test + void testBooleanConvertEdgeCases() { + final ExprEvaluator evaluator = new ExprEvaluator(EMPTY_MAP, EMPTY_MAP); + + assertEquals(true, evaluator.booleanConvert(" ")); + assertEquals(true, evaluator.booleanConvert(" ")); + assertEquals(true, evaluator.booleanConvert("\t")); + assertEquals(true, evaluator.booleanConvert("\n")); + + assertEquals(true, evaluator.booleanConvert("FALSE")); + assertEquals(true, evaluator.booleanConvert("False")); + assertEquals(true, evaluator.booleanConvert("TRUE")); + assertEquals(true, evaluator.booleanConvert("True")); + + assertEquals(false, evaluator.booleanConvert(0.1)); + assertEquals(false, evaluator.booleanConvert(-0.1)); + assertEquals(false, evaluator.booleanConvert(0.9)); + assertEquals(true, evaluator.booleanConvert(1.0)); + assertEquals(true, evaluator.booleanConvert(1.5)); + assertEquals(true, evaluator.booleanConvert(Long.MAX_VALUE)); + assertEquals(true, evaluator.booleanConvert(Long.MIN_VALUE)); + + assertEquals(false, evaluator.booleanConvert(0.0)); + assertEquals(false, evaluator.booleanConvert(-0.0)); + } } diff --git a/core-api/src/test/java/com/absmartly/sdk/jsonexpr/operators/EqualsOperatorTest.java b/core-api/src/test/java/com/absmartly/sdk/jsonexpr/operators/EqualsOperatorTest.java index 428eef9..88cd6cf 100644 --- a/core-api/src/test/java/com/absmartly/sdk/jsonexpr/operators/EqualsOperatorTest.java +++ b/core-api/src/test/java/com/absmartly/sdk/jsonexpr/operators/EqualsOperatorTest.java @@ -35,8 +35,7 @@ void testEvaluate() { Mockito.clearInvocations(evaluator); - assertNull(operator.evaluate(evaluator, listOf(null, null))); - verify(evaluator, Mockito.timeout(5000).times(1)).evaluate(any()); + assertTrue((Boolean) operator.evaluate(evaluator, listOf(null, null))); verify(evaluator, Mockito.timeout(5000).times(0)).compare(any(), any()); Mockito.clearInvocations(evaluator); diff --git a/core-api/src/test/java/com/absmartly/sdk/jsonexpr/operators/InOperatorTest.java b/core-api/src/test/java/com/absmartly/sdk/jsonexpr/operators/InOperatorTest.java index 9c07cde..85cb3e7 100644 --- a/core-api/src/test/java/com/absmartly/sdk/jsonexpr/operators/InOperatorTest.java +++ b/core-api/src/test/java/com/absmartly/sdk/jsonexpr/operators/InOperatorTest.java @@ -15,34 +15,20 @@ class InOperatorTest extends OperatorTest { @Test void testString() { - assertTrue((Boolean) operator.evaluate(evaluator, listOf("abcdefghijk", "abc"))); - assertTrue((Boolean) operator.evaluate(evaluator, listOf("abcdefghijk", "def"))); - assertFalse((Boolean) operator.evaluate(evaluator, listOf("abcdefghijk", "xxx"))); - assertNull(operator.evaluate(evaluator, listOf("abcdefghijk", null))); - assertNull(operator.evaluate(evaluator, listOf(null, "abc"))); - - verify(evaluator, Mockito.timeout(5000).times(4)).evaluate("abcdefghijk"); - verify(evaluator, Mockito.timeout(5000).times(1)).evaluate("abc"); - verify(evaluator, Mockito.timeout(5000).times(1)).evaluate("def"); - verify(evaluator, Mockito.timeout(5000).times(1)).evaluate("xxx"); - - verify(evaluator, Mockito.timeout(5000).times(1)).stringConvert("abc"); - verify(evaluator, Mockito.timeout(5000).times(1)).stringConvert("def"); - verify(evaluator, Mockito.timeout(5000).times(1)).stringConvert("xxx"); + assertTrue((Boolean) operator.evaluate(evaluator, listOf("abc", "abcdefghijk"))); + assertTrue((Boolean) operator.evaluate(evaluator, listOf("def", "abcdefghijk"))); + assertFalse((Boolean) operator.evaluate(evaluator, listOf("xxx", "abcdefghijk"))); + assertNull(operator.evaluate(evaluator, listOf(null, "abcdefghijk"))); + assertNull(operator.evaluate(evaluator, listOf("abc", null))); } @Test void testArrayEmpty() { - assertFalse((Boolean) operator.evaluate(evaluator, listOf(listOf(), 1))); - assertFalse((Boolean) operator.evaluate(evaluator, listOf(listOf(), "1"))); - assertFalse((Boolean) operator.evaluate(evaluator, listOf(listOf(), true))); - assertFalse((Boolean) operator.evaluate(evaluator, listOf(listOf(), false))); - assertNull(operator.evaluate(evaluator, listOf(listOf(), null))); - - verify(evaluator, Mockito.timeout(5000).times(0)).booleanConvert(any()); - verify(evaluator, Mockito.timeout(5000).times(0)).numberConvert(any()); - verify(evaluator, Mockito.timeout(5000).times(0)).stringConvert(any()); - verify(evaluator, Mockito.timeout(5000).times(0)).compare(any(), any()); + assertFalse((Boolean) operator.evaluate(evaluator, listOf(1, listOf()))); + assertFalse((Boolean) operator.evaluate(evaluator, listOf("1", listOf()))); + assertFalse((Boolean) operator.evaluate(evaluator, listOf(true, listOf()))); + assertFalse((Boolean) operator.evaluate(evaluator, listOf(false, listOf()))); + assertNull(operator.evaluate(evaluator, listOf(null, listOf()))); } @Test @@ -50,35 +36,19 @@ void testArrayCompares() { final List haystack01 = listOf(0, 1); final List haystack12 = listOf(1, 2); - assertFalse((Boolean) operator.evaluate(evaluator, listOf(haystack01, 2))); - verify(evaluator, Mockito.timeout(5000).times(2)).evaluate(any()); - verify(evaluator, Mockito.timeout(5000).times(1)).evaluate(haystack01); - verify(evaluator, Mockito.timeout(5000).times(1)).evaluate(2); - verify(evaluator, Mockito.timeout(5000).times(2)).compare(anyInt(), ArgumentMatchers.eq(2)); + assertFalse((Boolean) operator.evaluate(evaluator, listOf(2, haystack01))); Mockito.clearInvocations(evaluator); - assertFalse((Boolean) operator.evaluate(evaluator, listOf(haystack12, 0))); - verify(evaluator, Mockito.timeout(5000).times(2)).evaluate(any()); - verify(evaluator, Mockito.timeout(5000).times(1)).evaluate(haystack12); - verify(evaluator, Mockito.timeout(5000).times(1)).evaluate(0); - verify(evaluator, Mockito.timeout(5000).times(2)).compare(anyInt(), ArgumentMatchers.eq(0)); + assertFalse((Boolean) operator.evaluate(evaluator, listOf(0, haystack12))); Mockito.clearInvocations(evaluator); - assertTrue((Boolean) operator.evaluate(evaluator, listOf(haystack12, 1))); - verify(evaluator, Mockito.timeout(5000).times(2)).evaluate(any()); - verify(evaluator, Mockito.timeout(5000).times(1)).evaluate(haystack12); - verify(evaluator, Mockito.timeout(5000).times(1)).evaluate(1); - verify(evaluator, Mockito.timeout(5000).times(1)).compare(anyInt(), ArgumentMatchers.eq(1)); + assertTrue((Boolean) operator.evaluate(evaluator, listOf(1, haystack12))); Mockito.clearInvocations(evaluator); - assertTrue((Boolean) operator.evaluate(evaluator, listOf(haystack12, 2))); - verify(evaluator, Mockito.timeout(5000).times(2)).evaluate(any()); - verify(evaluator, Mockito.timeout(5000).times(1)).evaluate(haystack12); - verify(evaluator, Mockito.timeout(5000).times(1)).evaluate(2); - verify(evaluator, Mockito.timeout(5000).times(2)).compare(anyInt(), ArgumentMatchers.eq(2)); + assertTrue((Boolean) operator.evaluate(evaluator, listOf(2, haystack12))); Mockito.clearInvocations(evaluator); } @@ -88,44 +58,19 @@ void testObject() { final Map haystackab = mapOf("a", 1, "b", 2); final Map haystackbc = mapOf("b", 2, "c", 3, "0", 100); - assertFalse((Boolean) operator.evaluate(evaluator, listOf(haystackab, "c"))); - verify(evaluator, Mockito.timeout(5000).times(2)).evaluate(any()); - verify(evaluator, Mockito.timeout(5000).times(1)).evaluate(haystackab); - verify(evaluator, Mockito.timeout(5000).times(1)).stringConvert(any()); - verify(evaluator, Mockito.timeout(5000).times(1)).stringConvert("c"); - verify(evaluator, Mockito.timeout(5000).times(1)).evaluate("c"); + assertFalse((Boolean) operator.evaluate(evaluator, listOf("c", haystackab))); Mockito.clearInvocations(evaluator); - assertFalse((Boolean) operator.evaluate(evaluator, listOf(haystackbc, "a"))); - verify(evaluator, Mockito.timeout(5000).times(2)).evaluate(any()); - verify(evaluator, Mockito.timeout(5000).times(1)).evaluate(haystackbc); - verify(evaluator, Mockito.timeout(5000).times(1)).stringConvert(any()); - verify(evaluator, Mockito.timeout(5000).times(1)).stringConvert("a"); - verify(evaluator, Mockito.timeout(5000).times(1)).evaluate("a"); + assertFalse((Boolean) operator.evaluate(evaluator, listOf("a", haystackbc))); Mockito.clearInvocations(evaluator); - assertTrue((Boolean) operator.evaluate(evaluator, listOf(haystackbc, "b"))); - verify(evaluator, Mockito.timeout(5000).times(2)).evaluate(any()); - verify(evaluator, Mockito.timeout(5000).times(1)).evaluate(haystackbc); - verify(evaluator, Mockito.timeout(5000).times(1)).stringConvert(any()); - verify(evaluator, Mockito.timeout(5000).times(1)).stringConvert("b"); - verify(evaluator, Mockito.timeout(5000).times(1)).evaluate("b"); + assertTrue((Boolean) operator.evaluate(evaluator, listOf("b", haystackbc))); Mockito.clearInvocations(evaluator); - assertTrue((Boolean) operator.evaluate(evaluator, listOf(haystackbc, "c"))); - verify(evaluator, Mockito.timeout(5000).times(2)).evaluate(any()); - verify(evaluator, Mockito.timeout(5000).times(1)).evaluate(haystackbc); - verify(evaluator, Mockito.timeout(5000).times(1)).stringConvert(any()); - verify(evaluator, Mockito.timeout(5000).times(1)).stringConvert("c"); - verify(evaluator, Mockito.timeout(5000).times(1)).evaluate("c"); + assertTrue((Boolean) operator.evaluate(evaluator, listOf("c", haystackbc))); Mockito.clearInvocations(evaluator); - assertTrue((Boolean) operator.evaluate(evaluator, listOf(haystackbc, 0))); - verify(evaluator, Mockito.timeout(5000).times(2)).evaluate(any()); - verify(evaluator, Mockito.timeout(5000).times(1)).evaluate(haystackbc); - verify(evaluator, Mockito.timeout(5000).times(1)).stringConvert(any()); - verify(evaluator, Mockito.timeout(5000).times(1)).stringConvert(0); - verify(evaluator, Mockito.timeout(5000).times(1)).evaluate(0); + assertTrue((Boolean) operator.evaluate(evaluator, listOf(0, haystackbc))); Mockito.clearInvocations(evaluator); } } diff --git a/core-api/src/test/java/com/absmartly/sdk/jsonexpr/operators/MatchOperatorFixTest.java b/core-api/src/test/java/com/absmartly/sdk/jsonexpr/operators/MatchOperatorFixTest.java new file mode 100644 index 0000000..8057977 --- /dev/null +++ b/core-api/src/test/java/com/absmartly/sdk/jsonexpr/operators/MatchOperatorFixTest.java @@ -0,0 +1,84 @@ +package com.absmartly.sdk.jsonexpr.operators; + +import static org.junit.jupiter.api.Assertions.*; + +import org.junit.jupiter.api.Test; + +class MatchOperatorFixTest extends OperatorTest { + final MatchOperator operator = new MatchOperator(); + + @Test + void testBoundedThreadPoolRejectsLongPatterns() { + StringBuilder longPattern = new StringBuilder(); + for (int i = 0; i < 1001; i++) { + longPattern.append("a"); + } + assertNull(operator.evaluate(evaluator, listOf("test", longPattern.toString()))); + } + + @Test + void testRejectsLongInputText() { + StringBuilder longText = new StringBuilder(); + for (int i = 0; i < 10001; i++) { + longText.append("a"); + } + assertNull(operator.evaluate(evaluator, listOf(longText.toString(), "abc"))); + } + + @Test + void testNormalMatchingStillWorks() { + assertTrue((Boolean) operator.evaluate(evaluator, listOf("abcdefghijk", "abc"))); + assertFalse((Boolean) operator.evaluate(evaluator, listOf("abcdefghijk", "xyz"))); + } + + @Test + void testInvalidRegexReturnsNull() { + assertNull(operator.evaluate(evaluator, listOf("test", "["))); + } + + @Test + void testNullArgumentsReturnNull() { + assertNull(operator.evaluate(evaluator, listOf(null, "abc"))); + assertNull(operator.evaluate(evaluator, listOf("abc", null))); + } + + @Test + void testInterruptibleCharSequenceBasicBehavior() { + MatchOperator.InterruptibleCharSequence seq = new MatchOperator.InterruptibleCharSequence("hello"); + assertEquals(5, seq.length()); + assertEquals('h', seq.charAt(0)); + assertEquals('e', seq.charAt(1)); + assertEquals("ell", seq.subSequence(1, 4).toString()); + assertEquals("hello", seq.toString()); + } + + @Test + void testInterruptibleCharSequenceThrowsOnInterrupt() { + MatchOperator.InterruptibleCharSequence seq = new MatchOperator.InterruptibleCharSequence("hello"); + Thread.currentThread().interrupt(); + try { + assertThrows(MatchOperator.InterruptibleCharSequence.InterruptedCharAccessException.class, + () -> seq.charAt(0)); + } finally { + Thread.interrupted(); + } + } + + @Test + void testPatternAtMaxLength() { + StringBuilder pattern = new StringBuilder(); + for (int i = 0; i < 1000; i++) { + pattern.append("a"); + } + assertNotNull(operator.evaluate(evaluator, listOf("aaa", pattern.toString()))); + } + + @Test + void testTextAtMaxLength() { + StringBuilder text = new StringBuilder(); + for (int i = 0; i < 10000; i++) { + text.append("a"); + } + assertTrue((Boolean) operator.evaluate(evaluator, listOf(text.toString(), "aaa"))); + } +} diff --git a/example/src/main/java/com/absmartly/sdk/Example.java b/example/src/main/java/com/absmartly/sdk/Example.java index 456f279..d7eae40 100644 --- a/example/src/main/java/com/absmartly/sdk/Example.java +++ b/example/src/main/java/com/absmartly/sdk/Example.java @@ -12,10 +12,10 @@ static public void main(String[] args) throws IOException { .setApplication(System.getenv("ABSMARTLY_APP")) .setEnvironment(System.getenv("ABSMARTLY_ENV")); - final ABSmartlyConfig sdkConfig = ABSmartlyConfig.create() + final ABsmartlyConfig sdkConfig = ABsmartlyConfig.create() .setClient(Client.create(clientConfig)); - final ABSmartly sdk = ABSmartly.create(sdkConfig); + final ABsmartly sdk = ABsmartly.create(sdkConfig); final ContextConfig contextConfig = ContextConfig.create() .setUnit("session_id", "bf06d8cb5d8137290c4abb64155584fbdb64d8") diff --git a/gradle/jacoco.gradle b/gradle/jacoco.gradle index db9a55a..4601f05 100644 --- a/gradle/jacoco.gradle +++ b/gradle/jacoco.gradle @@ -2,7 +2,7 @@ apply plugin: "jacoco" jacoco { - toolVersion = "0.8.6" + toolVersion = "0.8.12" } @@ -10,7 +10,7 @@ project.tasks.withType(JacocoReport) { group "Verification" reports { - xml.enabled = true - html.enabled = true + xml.required = true + html.required = true } } diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index 790bdbf..c84565a 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -1,5 +1,4 @@ -#Thu May 28 18:38:14 CEST 2020 -distributionUrl=https\://services.gradle.org/distributions/gradle-5.6.4-all.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-8.5-all.zip distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists zipStorePath=wrapper/dists