From bc04e421462d9e6b2c0cdc5e8fbe8d7549657a9a Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Fri, 23 Oct 2026 09:37:43 +0100 Subject: [PATCH 01/14] feat!: reads throw before ready / after finalize (D1 parity with JS SDK) Align Context read methods with the JavaScript SDK _checkReady (javascript-sdk src/context.ts:533-541, experiments() at :439): - getTreatment, peekTreatment, getVariableValue, peekVariableValue, getVariableKeys, getCustomFieldKeys/Value/ValueType now call checkReady(true) and throw IllegalStateException before ready, while finalizing, or after finalize (previously returned defaults). - getExperiments uses checkReady(false): throws before ready but still works after finalize, like JS experiments(). - Messages: "ABsmartly Context is not yet ready.", "ABsmartly Context is finalizing.", "ABsmartly Context is finalized." - A context whose load failed stays ready+failed with empty data, so reads return defaults (new regression test). BREAKING: version 2.0.0; see CHANGELOG.md and README migration notes. --- CHANGELOG.md | 10 ++ README.md | 10 ++ build.gradle | 2 +- .../main/java/com/absmartly/sdk/Context.java | 46 ++----- .../java/com/absmartly/sdk/ContextTest.java | 128 +++++++++++------- 5 files changed, 115 insertions(+), 81 deletions(-) create mode 100644 CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..69da65d --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,10 @@ +# Changelog + +## 2.0.0 (unreleased) + +### BREAKING + +- Read methods throw `IllegalStateException` when the context is not ready or is finalized/finalizing, matching the JavaScript SDK `_checkReady` (maintainer decision D1). + - Old: `getTreatment`/`peekTreatment` returned `0`, `getVariableValue`/`peekVariableValue` returned the default value, `getVariableKeys` returned an empty map, `getExperiments`/`getCustomFieldKeys` returned an empty array, `getCustomFieldValue`/`getCustomFieldValueType` returned `null`. + - New: they throw `"ABsmartly Context is not yet ready."` before ready, and (except `getExperiments`, which mirrors JS `experiments()`) `"ABsmartly Context is finalizing."` / `"ABsmartly Context is finalized."` while closing / after close. +- Error message casing changed from `ABSmartly Context ...` to `ABsmartly Context ...`, and `"ABSmartly Context is closing."` became `"ABsmartly Context is finalizing."` to match the JavaScript SDK. diff --git a/README.md b/README.md index 4696568..cc1c8a2 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,16 @@ The `android.permission.INTERNET` permission is required. To add this permission If you target Android 6.0 or earlier, a few extra steps are outlined below for installation and initialization. +## Migrating to 2.0.0 (BREAKING) + +Read methods now throw `IllegalStateException` instead of returning defaults when the context is not ready, or after it is finalized (matching the JavaScript SDK): + +- `getTreatment`, `peekTreatment`, `getVariableValue`, `peekVariableValue`, `getVariableKeys`, `getCustomFieldKeys`, `getCustomFieldValue`, `getCustomFieldValueType` throw `"ABsmartly Context is not yet ready."` before ready, `"ABsmartly Context is finalizing."` while closing and `"ABsmartly Context is finalized."` after close. +- `getExperiments` throws `"ABsmartly Context is not yet ready."` before ready; like JS `experiments()`, it still works after finalize. +- All error messages now use the `ABsmartly` casing (was `ABSmartly`), and the closing message is now `"ABsmartly Context is finalizing."` (was `"ABSmartly Context is closing."`). + +Wait for the context before reading, e.g. `context.waitUntilReady()`, and check `context.isReady()` / `context.isFailed()` if you need a fallback. + ## Installation #### Gradle diff --git a/build.gradle b/build.gradle index 746a4d1..4a14d46 100644 --- a/build.gradle +++ b/build.gradle @@ -12,7 +12,7 @@ plugins { ext { - VERSION = "1.6.3" + VERSION = "2.0.0" GROUP_ID = "com.absmartly.sdk" slf4jVersion = "1.7.30" 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 389719b..6f68e2b 100644 --- a/core-api/src/main/java/com/absmartly/sdk/Context.java +++ b/core-api/src/main/java/com/absmartly/sdk/Context.java @@ -185,9 +185,7 @@ public Context waitUntilReady() { } public String[] getExperiments() { - if (!isReady() || isClosed() || isClosing()) { - return new String[0]; - } + checkReady(false); try { dataLock_.readLock().lock(); @@ -205,9 +203,7 @@ public String[] getExperiments() { } public String[] getCustomFieldKeys() { - if (!isReady() || isClosed() || isClosing()) { - return new String[0]; - } + checkReady(true); final Set keys = new HashSet(); @@ -228,9 +224,7 @@ public String[] getCustomFieldKeys() { } public Object getCustomFieldValue(@Nonnull final String experimentName, @Nonnull final String key) { - if (!isReady() || isClosed() || isClosing()) { - return null; - } + checkReady(true); try { dataLock_.readLock().lock(); @@ -248,9 +242,7 @@ 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; - } + checkReady(true); try { dataLock_.readLock().lock(); @@ -445,13 +437,7 @@ public void setAttributes(@Nonnull final Map attributes) { } public int getTreatment(@Nonnull final String experimentName) { - if (!isReady()) { - return 0; - } - - if (isClosed() || isClosing()) { - return 0; - } + checkReady(true); return getExposedTreatmentVariant(getAssignment(experimentName)); } @@ -584,17 +570,13 @@ private void enqueueExposure(final Assignment assignment) { } public int peekTreatment(@Nonnull final String experimentName) { - if (!isReady() || isClosed() || isClosing()) { - return 0; - } + checkReady(true); return getAssignment(experimentName).variant; } public Map> getVariableKeys() { - if (!isReady() || isClosed() || isClosing()) { - return new HashMap>(); - } + checkReady(true); final Map> variableKeys = new HashMap>(indexVariables_.size()); @@ -617,9 +599,7 @@ public Map> getVariableKeys() { } public Object getVariableValue(@Nonnull final String key, final Object defaultValue) { - if (!isReady() || isClosed() || isClosing()) { - return defaultValue; - } + checkReady(true); final Assignment assignment = exposeVariableAssignment(key, getVariableAssignment(key, false)); if (assignment != null) { @@ -657,9 +637,7 @@ private Assignment settleExposure(final Assignment assignment, final Function(), 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")); + assertEquals(notReadyMessage, + assertThrows(IllegalStateException.class, () -> context.peekTreatment("exp_test_ab")).getMessage()); + assertEquals(notReadyMessage, + assertThrows(IllegalStateException.class, () -> context.getTreatment("exp_test_ab")).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(notReadyMessage, + assertThrows(IllegalStateException.class, context::getCustomFieldKeys).getMessage()); + assertEquals(notReadyMessage, + assertThrows(IllegalStateException.class, () -> context.getCustomFieldValue("exp_test_ab", "key")) + .getMessage()); + assertEquals(notReadyMessage, + assertThrows(IllegalStateException.class, () -> context.getCustomFieldValueType("exp_test_ab", "key")) + .getMessage()); } @Test @@ -395,7 +407,7 @@ void throwsWhenClosing() { assertTrue(context.isClosing()); assertFalse(context.isClosed()); - final String closingMessage = "ABSmartly Context is closing."; + final String closingMessage = "ABsmartly Context is finalizing."; assertEquals(closingMessage, assertThrows(IllegalStateException.class, () -> context.setAttribute("attr1", "value1")).getMessage()); assertEquals(closingMessage, @@ -417,15 +429,27 @@ void throwsWhenClosing() { assertEquals(closingMessage, assertThrows(IllegalStateException.class, context::publish).getMessage()); assertEquals(closingMessage, assertThrows(IllegalStateException.class, context::getData).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")); + assertEquals(closingMessage, + assertThrows(IllegalStateException.class, () -> context.peekTreatment("exp_test_ab")).getMessage()); + assertEquals(closingMessage, + assertThrows(IllegalStateException.class, () -> context.getTreatment("exp_test_ab")).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(closingMessage, + assertThrows(IllegalStateException.class, context::getCustomFieldKeys).getMessage()); + assertEquals(closingMessage, + assertThrows(IllegalStateException.class, () -> context.getCustomFieldValue("exp_test_ab", "key")) + .getMessage()); + assertEquals(closingMessage, + assertThrows(IllegalStateException.class, () -> context.getCustomFieldValueType("exp_test_ab", "key")) + .getMessage()); + // matches JS experiments(): _checkReady() without the finalized check + assertEquals(data.experiments.length, context.getExperiments().length); } @Test @@ -443,7 +467,7 @@ void throwsWhenClosed() { assertFalse(context.isClosing()); assertTrue(context.isClosed()); - final String closedMessage = "ABSmartly Context is finalized."; + final String closedMessage = "ABsmartly Context is finalized."; assertEquals(closedMessage, assertThrows(IllegalStateException.class, () -> context.setAttribute("attr1", "value1")).getMessage()); assertEquals(closedMessage, @@ -465,15 +489,27 @@ void throwsWhenClosed() { assertEquals(closedMessage, assertThrows(IllegalStateException.class, context::publish).getMessage()); assertEquals(closedMessage, assertThrows(IllegalStateException.class, context::getData).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")); + assertEquals(closedMessage, + assertThrows(IllegalStateException.class, () -> context.peekTreatment("exp_test_ab")).getMessage()); + assertEquals(closedMessage, + assertThrows(IllegalStateException.class, () -> context.getTreatment("exp_test_ab")).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(closedMessage, + assertThrows(IllegalStateException.class, context::getCustomFieldKeys).getMessage()); + assertEquals(closedMessage, + assertThrows(IllegalStateException.class, () -> context.getCustomFieldValue("exp_test_ab", "key")) + .getMessage()); + assertEquals(closedMessage, + assertThrows(IllegalStateException.class, () -> context.getCustomFieldValueType("exp_test_ab", "key")) + .getMessage()); + // matches JS experiments(): _checkReady() without the finalized check + assertEquals(data.experiments.length, context.getExperiments().length); } @Test From 1e3b20e91dbf4d2568f673666d546c904ae8132d Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Tue, 27 Oct 2026 13:10:17 +0000 Subject: [PATCH 02/14] fix: keep numeric unit types, log lifecycle events before settling futures, no-op refresh on failed context - Units accept numbers as well as strings and getUnit/getUnits return the original type. Numeric UIDs hash as the value rounded to an integer, like the JavaScript SDK's toFixed(0). - Ready, Refresh, Error and Close events are logged before the matching future completes, so callers waiting on the future see the event. - refresh() on a context whose initial load failed does nothing and succeeds, matching the JavaScript SDK. --- CHANGELOG.md | 6 + .../main/java/com/absmartly/sdk/Context.java | 119 +++++++----- .../java/com/absmartly/sdk/ContextConfig.java | 30 +-- .../sdk/internal/hashing/Hashing.java | 28 +++ .../java/com/absmartly/sdk/ContextTest.java | 172 ++++++++++++++++-- 5 files changed, 287 insertions(+), 68 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 69da65d..320687b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,3 +8,9 @@ - Old: `getTreatment`/`peekTreatment` returned `0`, `getVariableValue`/`peekVariableValue` returned the default value, `getVariableKeys` returned an empty map, `getExperiments`/`getCustomFieldKeys` returned an empty array, `getCustomFieldValue`/`getCustomFieldValueType` returned `null`. - New: they throw `"ABsmartly Context is not yet ready."` before ready, and (except `getExperiments`, which mirrors JS `experiments()`) `"ABsmartly Context is finalizing."` / `"ABsmartly Context is finalized."` while closing / after close. - Error message casing changed from `ABSmartly Context ...` to `ABsmartly Context ...`, and `"ABSmartly Context is closing."` became `"ABsmartly Context is finalizing."` to match the JavaScript SDK. +- Unit UIDs can be strings or numbers, like the JavaScript SDK. `ContextConfig.setUnit`/`Context.setUnit` accept a `Number`, `getUnit` returns `Object` and `getUnits`/`ContextConfig.getUnits` return `Map`, keeping the type the UID was set with. A numeric UID is hashed as its value rounded to an integer (`125.75` hashes as `"126"`), as JavaScript's `toFixed(0)` does. A blank UID is now reported as blank even when the unit type is already set. + +### Changed + +- The `Ready`, `Refresh`, `Error` and `Close` events are logged before the matching future (`waitUntilReadyAsync`, `refreshAsync`, `closeAsync`) completes, as in the JavaScript SDK, so a caller that waits on the future always sees the event. +- `refresh()` on a context whose initial load failed is a successful no-op: it does not fetch and logs no event, as in the JavaScript SDK. 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 6f68e2b..e602a72 100644 --- a/core-api/src/main/java/com/absmartly/sdk/Context.java +++ b/core-api/src/main/java/com/absmartly/sdk/Context.java @@ -61,9 +61,9 @@ private Context(Clock clock, ContextConfig config, ScheduledExecutorService sche audienceMatcher_ = audienceMatcher; scheduler_ = scheduler; - units_ = new HashMap(); + units_ = new HashMap(); - final Map units = config.getUnits(); + final Map units = config.getUnits(); if (units != null) { setUnits(units); } @@ -105,13 +105,15 @@ public Void apply(Throwable exception) { @Override public void accept(ContextData data) { Context.this.setData(data); - final CompletableFuture rf = readyFuture_.getAndSet(COMPLETED_VOID_FUTURE); - if (rf != null) { - rf.complete(null); + try { + Context.this.logEvent(ContextEventLogger.EventType.Ready, data); + } finally { + final CompletableFuture rf = readyFuture_.getAndSet(COMPLETED_VOID_FUTURE); + if (rf != null) { + rf.complete(null); + } } - Context.this.logEvent(ContextEventLogger.EventType.Ready, data); - if (Context.this.getPendingCount() > 0) { Context.this.setTimeout(); } @@ -120,13 +122,15 @@ public void accept(ContextData data) { @Override public Void apply(Throwable exception) { Context.this.setDataFailed(exception); - final CompletableFuture rf = readyFuture_.getAndSet(COMPLETED_VOID_FUTURE); - if (rf != null) { - rf.complete(null); + try { + Context.this.logError(exception); + } finally { + final CompletableFuture rf = readyFuture_.getAndSet(COMPLETED_VOID_FUTURE); + if (rf != null) { + rf.complete(null); + } } - Context.this.logError(exception); - return null; } }); @@ -304,7 +308,7 @@ public void setCustomAssignments(@Nonnull final Map customAssig } } - public String getUnit(@Nonnull final String unitType) { + public Object getUnit(@Nonnull final String unitType) { final ReentrantReadWriteLock.ReadLock readLock = contextLock_.readLock(); try { readLock.lock(); @@ -315,23 +319,35 @@ public String getUnit(@Nonnull final String unitType) { } public void setUnit(@Nonnull final String unitType, @Nonnull final String uid) { + putUnit(unitType, uid); + } + + public void setUnit(@Nonnull final String unitType, @Nonnull final Number uid) { + putUnit(unitType, uid); + } + + private void putUnit(final String unitType, Object uid) { checkNotClosed(); + if (uid instanceof String) { + uid = ((String) uid).trim(); + if (((String) uid).isEmpty()) { + throw new IllegalArgumentException(String.format("Unit '%s' UID must not be blank.", unitType)); + } + } else if (!(uid instanceof Number)) { + throw new IllegalArgumentException(String.format("Unit '%s' must be a string or a number.", unitType)); + } + final ReentrantReadWriteLock.WriteLock writeLock = contextLock_.writeLock(); try { writeLock.lock(); - final String previous = units_.get(unitType); - if ((previous != null) && !previous.equals(uid)) { + final Object previous = units_.get(unitType); + if ((previous != null) && !sameUid(previous, uid)) { throw new IllegalArgumentException(String.format("Unit '%s' UID already set.", unitType)); } - final String trimmed = uid.trim(); - if (trimmed.isEmpty()) { - throw new IllegalArgumentException(String.format("Unit '%s' UID must not be blank.", unitType)); - } - - units_.put(unitType, trimmed); + units_.put(unitType, uid); invalidateAssignmentsPinnedWithMissingUnit(unitType); } finally { @@ -339,6 +355,13 @@ public void setUnit(@Nonnull final String unitType, @Nonnull final String uid) { } } + private static boolean sameUid(final Object a, final Object b) { + if ((a instanceof Number) && (b instanceof Number)) { + return ((Number) a).doubleValue() == ((Number) b).doubleValue(); + } + return a.equals(b); + } + // holdoutAssignments pins the holdout decisions used to compute suppression. A null entry means // the covered experiment's unit was unavailable; resolving it live later could publish a holdout // verdict inconsistent with the cached experiment decision, so setUnit evicts the assignment and @@ -372,21 +395,19 @@ private void invalidateAssignmentsPinnedWithMissingUnit(final String unitType) { } } - public Map getUnits() { + public Map getUnits() { final ReentrantReadWriteLock.ReadLock readLock = contextLock_.readLock(); try { readLock.lock(); - return new HashMap(units_); + return new HashMap(units_); } finally { readLock.unlock(); } } - public void setUnits(@Nonnull final Map units) { - for (Map.Entry entry : units.entrySet()) { - String key = entry.getKey(); - String value = entry.getValue(); - setUnit(key, value); + public void setUnits(@Nonnull final Map units) { + for (Map.Entry entry : units.entrySet()) { + putUnit(entry.getKey(), entry.getValue()); } } @@ -688,6 +709,10 @@ public int getPendingCount() { public CompletableFuture refreshAsync() { checkNotClosed(); + if (failed_) { + return CompletableFuture.completedFuture(null); + } + if (refreshing_.compareAndSet(false, true)) { final CompletableFuture newRefreshFuture = new CompletableFuture(); refreshFuture_.set(newRefreshFuture); @@ -697,17 +722,21 @@ public CompletableFuture refreshAsync() { public void accept(ContextData data) { Context.this.setData(data); refreshing_.set(false); - newRefreshFuture.complete(null); - - Context.this.logEvent(ContextEventLogger.EventType.Refresh, data); + try { + Context.this.logEvent(ContextEventLogger.EventType.Refresh, data); + } finally { + newRefreshFuture.complete(null); + } } }).exceptionally(new Function() { @Override public Void apply(Throwable exception) { refreshing_.set(false); - newRefreshFuture.completeExceptionally(exception); - - Context.this.logError(exception); + try { + Context.this.logError(exception); + } finally { + newRefreshFuture.completeExceptionally(exception); + } return null; } }); @@ -739,9 +768,11 @@ public CompletableFuture closeAsync() { public void accept(Void x) { closed_.set(true); closing_.set(false); - newClosingFuture.complete(null); - - Context.this.logEvent(ContextEventLogger.EventType.Close, null); + try { + Context.this.logEvent(ContextEventLogger.EventType.Close, null); + } finally { + newClosingFuture.complete(null); + } } }).exceptionally(new Function() { @Override @@ -842,9 +873,9 @@ public Void apply(Void ignoredResult, Throwable exception) { try { contextLock_.writeLock().lock(); event.units = Algorithm.mapSetToArray(units_.entrySet(), new Unit[0], - new Function, Unit>() { + new Function, Unit>() { @Override - public Unit apply(Map.Entry entry) { + public Unit apply(Map.Entry entry) { return new Unit(entry.getKey(), new String(getUnitHash(entry.getKey(), entry.getValue()), StandardCharsets.US_ASCII)); @@ -1230,7 +1261,7 @@ && audienceMatches(experiment.data, assignment)) { if (experiment.data.audienceStrict && assignment.audienceMismatch) { assignment.variant = 0; } else if (experiment.data.fullOnVariant == 0) { - final String uid = units_.get(unitType); + final Object uid = units_.get(unitType); if (uid != null) { final byte[] unitHash = Context.this.getUnitHash(unitType, uid); @@ -1392,7 +1423,7 @@ private Assignment getHoldoutAssignment(final Experiment holdout, final String u try { readLock.lock(); - final String uid = units_.get(unitType); + final Object uid = units_.get(unitType); if (uid == null) { return null; } @@ -1412,7 +1443,7 @@ private Assignment getHoldoutAssignment(final Experiment holdout, final String u try { writeLock.lock(); - final String uid = units_.get(unitType); + final Object uid = units_.get(unitType); if (uid == null) { return null; } @@ -1457,7 +1488,7 @@ private List getVariableExperiments(final String key) { return Concurrency.getRW(dataLock_, indexVariables_, key); } - private byte[] getUnitHash(final String unitType, final String unitUID) { + private byte[] getUnitHash(final String unitType, final Object unitUID) { return Concurrency.computeIfAbsentRW(contextLock_, hashedUnits_, unitType, new Function() { @Override public byte[] apply(String key) { @@ -1742,7 +1773,7 @@ private Map buildAttributesMap() { private final VariableParser variableParser_; private final AudienceMatcher audienceMatcher_; private final ScheduledExecutorService scheduler_; - private final Map units_; + private final Map units_; private volatile boolean failed_; private volatile Throwable readyError_; 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 a6ecbfc..56d1e95 100644 --- a/core-api/src/main/java/com/absmartly/sdk/ContextConfig.java +++ b/core-api/src/main/java/com/absmartly/sdk/ContextConfig.java @@ -13,29 +13,37 @@ public static ContextConfig create() { private ContextConfig() {} public ContextConfig setUnit(@Nonnull final String unitType, @Nonnull final String uid) { - if (units_ == null) { - units_ = new HashMap(); - } + return putUnit(unitType, uid); + } - units_.put(unitType, uid); - return this; + public ContextConfig setUnit(@Nonnull final String unitType, @Nonnull final Number uid) { + return putUnit(unitType, uid); } - public ContextConfig setUnits(@Nonnull final Map units) { - for (final Map.Entry entry : units.entrySet()) { - setUnit(entry.getKey(), entry.getValue()); + public ContextConfig setUnits(@Nonnull final Map units) { + for (final Map.Entry entry : units.entrySet()) { + putUnit(entry.getKey(), entry.getValue()); } return this; } - public String getUnit(@Nonnull final String unitType) { + public Object getUnit(@Nonnull final String unitType) { return units_ != null ? units_.get(unitType) : null; } - public Map getUnits() { + public Map getUnits() { return units_; } + private ContextConfig putUnit(final String unitType, final Object uid) { + if (units_ == null) { + units_ = new HashMap(); + } + + units_.put(unitType, uid); + return this; + } + public ContextConfig setAttribute(@Nonnull final String name, @Nonnull final Object value) { if (attributes_ == null) { attributes_ = new HashMap(); @@ -135,7 +143,7 @@ public long getRefreshInterval() { return this.refreshInterval_; } - private Map units_; + private Map units_; private Map attributes_; private Map overrides_; private Map cassignments_; diff --git a/core-api/src/main/java/com/absmartly/sdk/internal/hashing/Hashing.java b/core-api/src/main/java/com/absmartly/sdk/internal/hashing/Hashing.java index 49823a8..d755375 100644 --- a/core-api/src/main/java/com/absmartly/sdk/internal/hashing/Hashing.java +++ b/core-api/src/main/java/com/absmartly/sdk/internal/hashing/Hashing.java @@ -1,5 +1,9 @@ package com.absmartly.sdk.internal.hashing; +import java.math.BigDecimal; +import java.math.BigInteger; +import java.math.RoundingMode; + import com.absmartly.sdk.internal.Buffers; public abstract class Hashing { @@ -12,6 +16,30 @@ public byte[] initialValue() { } }; + public static byte[] hashUnit(Object unit) { + return hashUnit(unitToString(unit)); + } + + // Numeric unit UIDs hash like the JavaScript SDK's value.toFixed(0): rounded to an integer, no + // decimal point. + public static String unitToString(final Object uid) { + if (uid instanceof String) { + return (String) uid; + } + if ((uid instanceof Integer) || (uid instanceof Long) || (uid instanceof Short) || (uid instanceof Byte) + || (uid instanceof BigInteger)) { + return uid.toString(); + } + if (uid instanceof BigDecimal) { + return ((BigDecimal) uid).setScale(0, RoundingMode.HALF_UP).toPlainString(); + } + final double value = ((Number) uid).doubleValue(); + if (Double.isNaN(value) || Double.isInfinite(value)) { + return value > 0 ? "Infinity" : (value < 0 ? "-Infinity" : "NaN"); + } + return new BigDecimal(value).setScale(0, RoundingMode.HALF_UP).toPlainString(); + } + public static byte[] hashUnit(CharSequence unit) { final int n = unit.length(); // Up to 4 UTF-8 bytes per UTF-16 code unit (3-byte BMP chars, and 4-byte 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 6b8e2d5..928dc13 100644 --- a/core-api/src/test/java/com/absmartly/sdk/ContextTest.java +++ b/core-api/src/test/java/com/absmartly/sdk/ContextTest.java @@ -39,6 +39,7 @@ import org.mockito.ArgumentMatchers; import org.mockito.Mockito; +import com.absmartly.sdk.internal.hashing.Hashing; import com.absmartly.sdk.java.time.Clock; import com.absmartly.sdk.json.Attribute; import com.absmartly.sdk.json.ContextData; @@ -2525,19 +2526,6 @@ void refreshKeepsCustomAssignments() { 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(); @@ -3075,4 +3063,162 @@ void cancellingPublishResultDoesNotDisturbPublishAccounting() { retryPublish.join(); assertEquals(0, context.getPendingCount()); } + + private void recordFutureStateOnEvent(final ContextEventLogger.EventType type, + final AtomicReference> future, final AtomicReference doneWhenLogged) { + Mockito.doAnswer(invocation -> { + if (invocation.getArgument(1) == type && future.get() != null) { + doneWhenLogged.set(future.get().isDone()); + } + return null; + }).when(eventLogger).handleEvent(any(), any(), any()); + } + + @Test + void readyEventIsLoggedBeforeReadyFutureSettles() { + final AtomicReference> future = new AtomicReference<>(); + final AtomicReference doneWhenLogged = new AtomicReference<>(); + recordFutureStateOnEvent(ContextEventLogger.EventType.Ready, future, doneWhenLogged); + + final Context context = createContext(dataFuture); + future.set(context.waitUntilReadyAsync()); + dataFuture.complete(data); + + assertEquals(Boolean.FALSE, doneWhenLogged.get()); + assertTrue(future.get().isDone()); + } + + @Test + void errorEventIsLoggedBeforeReadyFutureSettlesOnFailedLoad() { + final AtomicReference> future = new AtomicReference<>(); + final AtomicReference doneWhenLogged = new AtomicReference<>(); + recordFutureStateOnEvent(ContextEventLogger.EventType.Error, future, doneWhenLogged); + + final Context context = createContext(dataFuture); + future.set(context.waitUntilReadyAsync()); + dataFuture.completeExceptionally(new Exception("FAILED")); + + assertEquals(Boolean.FALSE, doneWhenLogged.get()); + assertTrue(context.isFailed()); + } + + @Test + void refreshEventIsLoggedBeforeRefreshFutureSettles() { + final AtomicReference> future = new AtomicReference<>(); + final AtomicReference doneWhenLogged = new AtomicReference<>(); + recordFutureStateOnEvent(ContextEventLogger.EventType.Refresh, future, doneWhenLogged); + + final Context context = createReadyContext(); + when(dataProvider.getContextData()).thenReturn(refreshDataFuture); + future.set(context.refreshAsync()); + refreshDataFuture.complete(refreshData); + + assertEquals(Boolean.FALSE, doneWhenLogged.get()); + assertTrue(future.get().isDone()); + } + + @Test + void errorEventIsLoggedBeforeRefreshFutureSettlesOnFailure() { + final AtomicReference> future = new AtomicReference<>(); + final AtomicReference doneWhenLogged = new AtomicReference<>(); + recordFutureStateOnEvent(ContextEventLogger.EventType.Error, future, doneWhenLogged); + + final Context context = createReadyContext(); + when(dataProvider.getContextData()).thenReturn(refreshDataFuture); + future.set(context.refreshAsync()); + refreshDataFuture.completeExceptionally(new Exception("FAILED")); + + assertEquals(Boolean.FALSE, doneWhenLogged.get()); + assertTrue(future.get().isCompletedExceptionally()); + } + + @Test + void closeEventIsLoggedBeforeCloseFutureSettles() { + final AtomicReference> future = new AtomicReference<>(); + final AtomicReference doneWhenLogged = new AtomicReference<>(); + recordFutureStateOnEvent(ContextEventLogger.EventType.Close, future, doneWhenLogged); + + final Context context = createReadyContext(); + context.track("goal1", null); + final CompletableFuture publishFuture = new CompletableFuture<>(); + when(eventHandler.publish(any(), any())).thenReturn(publishFuture); + + future.set(context.closeAsync()); + publishFuture.complete(null); + future.get().join(); + + assertEquals(Boolean.FALSE, doneWhenLogged.get()); + assertTrue(context.isClosed()); + } + + @Test + void numericUnitsKeepTheirType() { + final ContextConfig config = ContextConfig.create() + .setUnit("session_id", "abc") + .setUnit("user_id", 125) + .setUnit("float_id", 125.75); + final Context context = createContext(config, dataFutureReady); + context.setUnit("org_id", 99L); + + assertEquals(125, context.getUnit("user_id")); + assertEquals(125.75, context.getUnit("float_id")); + assertEquals(99L, context.getUnit("org_id")); + assertEquals("abc", context.getUnit("session_id")); + assertEquals(mapOf("session_id", "abc", "user_id", 125, "float_id", 125.75, "org_id", 99L), + context.getUnits()); + } + + @Test + void numericUnitHashesLikeIntegerString() { + assertEquals("126", Hashing.unitToString(125.75)); + assertEquals("125", Hashing.unitToString(125)); + assertEquals("-3", Hashing.unitToString(-2.5)); + assertEquals("123456789", Hashing.unitToString(123456789L)); + + when(eventHandler.publish(any(), any())).thenReturn(CompletableFuture.completedFuture(null)); + final Context numeric = createContext(ContextConfig.create().setUnit("user_id", 125.75), dataFutureReady); + final Context text = createContext(ContextConfig.create().setUnit("user_id", "126"), dataFutureReady); + numeric.track("goal", null); + text.track("goal", null); + final List events = new ArrayList<>(); + Mockito.doAnswer(invocation -> { + events.add(invocation.getArgument(1)); + return CompletableFuture.completedFuture(null); + }).when(eventHandler).publish(any(), any()); + numeric.publish(); + text.publish(); + + assertEquals(2, events.size()); + assertEquals(events.get(1).units[0].uid, events.get(0).units[0].uid); + } + + @Test + void setUnitRejectsSameNumericValueOnlyWhenDifferent() { + final Context context = createContext(ContextConfig.create().setUnit("user_id", 12345), dataFutureReady); + assertDoesNotThrow(() -> context.setUnit("user_id", 12345)); + final IllegalArgumentException error = assertThrows(IllegalArgumentException.class, + () -> context.setUnit("user_id", 54321)); + assertEquals("Unit 'user_id' UID already set.", error.getMessage()); + } + + @Test + void setUnitChecksBlankBeforeAlreadySet() { + final Context context = createReadyContext(); + final IllegalArgumentException error = assertThrows(IllegalArgumentException.class, + () -> context.setUnit("user_id", " ")); + assertEquals("Unit 'user_id' UID must not be blank.", error.getMessage()); + } + + @Test + void refreshOnFailedContextIsANoOp() { + final Context context = createContext(dataFutureFailed); + assertTrue(context.isFailed()); + Mockito.clearInvocations(eventLogger); + + context.refresh(); + + verify(dataProvider, Mockito.never()).getContextData(); + Mockito.verifyNoInteractions(eventLogger); + assertTrue(context.isFailed()); + } } From f0505e2ea3ffe25691345152de74f40f633bf48b Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Tue, 27 Oct 2026 13:43:02 +0000 Subject: [PATCH 03/14] fix: stable numeric unit identities and JavaScript-compatible numeric hashing - Compare numeric UIDs exactly, so distinct large longs are not treated as the same UID. - Reject mutable Number types such as AtomicInteger: the context caches the unit hash, so a UID changing after it was set would publish under its old identity. - Format numeric UIDs exactly like JavaScript's toFixed(0): convert to double first, keep "-0", and use exponent form from 1e21. - Document that event loggers must not wait on the lifecycle future of the event they receive. --- CHANGELOG.md | 2 +- .../main/java/com/absmartly/sdk/Context.java | 33 +++++++++++++++++-- .../com/absmartly/sdk/ContextEventLogger.java | 6 ++++ .../sdk/internal/hashing/Hashing.java | 30 ++++++++++------- .../java/com/absmartly/sdk/ContextTest.java | 32 +++++++++++++++++- 5 files changed, 87 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 320687b..91ace6d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ - Old: `getTreatment`/`peekTreatment` returned `0`, `getVariableValue`/`peekVariableValue` returned the default value, `getVariableKeys` returned an empty map, `getExperiments`/`getCustomFieldKeys` returned an empty array, `getCustomFieldValue`/`getCustomFieldValueType` returned `null`. - New: they throw `"ABsmartly Context is not yet ready."` before ready, and (except `getExperiments`, which mirrors JS `experiments()`) `"ABsmartly Context is finalizing."` / `"ABsmartly Context is finalized."` while closing / after close. - Error message casing changed from `ABSmartly Context ...` to `ABsmartly Context ...`, and `"ABSmartly Context is closing."` became `"ABsmartly Context is finalizing."` to match the JavaScript SDK. -- Unit UIDs can be strings or numbers, like the JavaScript SDK. `ContextConfig.setUnit`/`Context.setUnit` accept a `Number`, `getUnit` returns `Object` and `getUnits`/`ContextConfig.getUnits` return `Map`, keeping the type the UID was set with. A numeric UID is hashed as its value rounded to an integer (`125.75` hashes as `"126"`), as JavaScript's `toFixed(0)` does. A blank UID is now reported as blank even when the unit type is already set. +- Unit UIDs can be strings or numbers, like the JavaScript SDK. `ContextConfig.setUnit`/`Context.setUnit` accept a `Number`, `getUnit` returns `Object` and `getUnits`/`ContextConfig.getUnits` return `Map`, keeping the type the UID was set with. A numeric UID is hashed as its value rounded to an integer (`125.75` hashes as `"126"`), as JavaScript's `toFixed(0)` does (including `"-0"` and `"1e+21"` forms). Mutable `Number` types such as `AtomicInteger` are rejected. A blank UID is now reported as blank even when the unit type is already set. ### Changed 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 e602a72..0587294 100644 --- a/core-api/src/main/java/com/absmartly/sdk/Context.java +++ b/core-api/src/main/java/com/absmartly/sdk/Context.java @@ -1,6 +1,8 @@ package com.absmartly.sdk; import java.io.Closeable; +import java.math.BigDecimal; +import java.math.BigInteger; import java.util.*; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; @@ -334,7 +336,7 @@ private void putUnit(final String unitType, Object uid) { if (((String) uid).isEmpty()) { throw new IllegalArgumentException(String.format("Unit '%s' UID must not be blank.", unitType)); } - } else if (!(uid instanceof Number)) { + } else if (!isImmutableNumber(uid)) { throw new IllegalArgumentException(String.format("Unit '%s' must be a string or a number.", unitType)); } @@ -355,13 +357,40 @@ private void putUnit(final String unitType, Object uid) { } } + // Mutable Number implementations (AtomicInteger, ...) are rejected: the context caches the unit's + // hash, so a UID that changes after it is set would be published under its old identity. + private static boolean isImmutableNumber(final Object uid) { + return (uid instanceof Integer) || (uid instanceof Long) || (uid instanceof Short) || (uid instanceof Byte) + || (uid instanceof Double) || (uid instanceof Float) || (uid instanceof BigInteger) + || (uid instanceof BigDecimal); + } + private static boolean sameUid(final Object a, final Object b) { if ((a instanceof Number) && (b instanceof Number)) { - return ((Number) a).doubleValue() == ((Number) b).doubleValue(); + final BigDecimal x = toBigDecimal((Number) a); + final BigDecimal y = toBigDecimal((Number) b); + if ((x == null) || (y == null)) { + return Double.valueOf(((Number) a).doubleValue()).equals(((Number) b).doubleValue()); + } + return x.compareTo(y) == 0; } return a.equals(b); } + private static BigDecimal toBigDecimal(final Number value) { + if (value instanceof BigDecimal) { + return (BigDecimal) value; + } + if (value instanceof BigInteger) { + return new BigDecimal((BigInteger) value); + } + if ((value instanceof Double) || (value instanceof Float)) { + final double d = value.doubleValue(); + return (Double.isNaN(d) || Double.isInfinite(d)) ? null : new BigDecimal(d); + } + return BigDecimal.valueOf(value.longValue()); + } + // holdoutAssignments pins the holdout decisions used to compute suppression. A null entry means // the covered experiment's unit was unavailable; resolving it live later could publish a holdout // verdict inconsistent with the cached experiment decision, so setUnit evicts the assignment and diff --git a/core-api/src/main/java/com/absmartly/sdk/ContextEventLogger.java b/core-api/src/main/java/com/absmartly/sdk/ContextEventLogger.java index 5b98fda..e030ec3 100644 --- a/core-api/src/main/java/com/absmartly/sdk/ContextEventLogger.java +++ b/core-api/src/main/java/com/absmartly/sdk/ContextEventLogger.java @@ -5,5 +5,11 @@ enum EventType { Error, Ready, Refresh, Publish, Exposure, Goal, Close } + /** + * Called synchronously by the context. Ready, Refresh, Close and Error events are delivered + * before the matching future ({@code waitUntilReadyAsync}, {@code refreshAsync}, + * {@code closeAsync}) completes, so an implementation must not wait on that future: it would + * block forever. + */ void handleEvent(Context context, EventType type, Object data); } diff --git a/core-api/src/main/java/com/absmartly/sdk/internal/hashing/Hashing.java b/core-api/src/main/java/com/absmartly/sdk/internal/hashing/Hashing.java index d755375..6af815d 100644 --- a/core-api/src/main/java/com/absmartly/sdk/internal/hashing/Hashing.java +++ b/core-api/src/main/java/com/absmartly/sdk/internal/hashing/Hashing.java @@ -1,7 +1,6 @@ package com.absmartly.sdk.internal.hashing; import java.math.BigDecimal; -import java.math.BigInteger; import java.math.RoundingMode; import com.absmartly.sdk.internal.Buffers; @@ -20,24 +19,31 @@ public static byte[] hashUnit(Object unit) { return hashUnit(unitToString(unit)); } - // Numeric unit UIDs hash like the JavaScript SDK's value.toFixed(0): rounded to an integer, no - // decimal point. + // Numeric unit UIDs hash like the JavaScript SDK's value.toFixed(0). JavaScript numbers are doubles, + // so every numeric type is converted to double first (a long beyond 2^53 rounds as it would in + // JavaScript), negative values that round to zero keep their sign ("-0"), and magnitudes of 1e21 + // or more use JavaScript's exponent notation ("1e+21"). public static String unitToString(final Object uid) { if (uid instanceof String) { return (String) uid; } - if ((uid instanceof Integer) || (uid instanceof Long) || (uid instanceof Short) || (uid instanceof Byte) - || (uid instanceof BigInteger)) { - return uid.toString(); + final double value = ((Number) uid).doubleValue(); + if (Double.isNaN(value)) { + return "NaN"; } - if (uid instanceof BigDecimal) { - return ((BigDecimal) uid).setScale(0, RoundingMode.HALF_UP).toPlainString(); + if (Double.isInfinite(value)) { + return value > 0 ? "Infinity" : "-Infinity"; } - final double value = ((Number) uid).doubleValue(); - if (Double.isNaN(value) || Double.isInfinite(value)) { - return value > 0 ? "Infinity" : (value < 0 ? "-Infinity" : "NaN"); + final double magnitude = Math.abs(value); + final String sign = value < 0 ? "-" : ""; + if (magnitude >= 1e21) { + final BigDecimal shortest = new BigDecimal(Double.toString(magnitude)).stripTrailingZeros(); + final String digits = shortest.unscaledValue().toString(); + final int exponent = digits.length() - 1 - shortest.scale(); + final String mantissa = digits.length() == 1 ? digits : digits.charAt(0) + "." + digits.substring(1); + return sign + mantissa + "e+" + exponent; } - return new BigDecimal(value).setScale(0, RoundingMode.HALF_UP).toPlainString(); + return sign + new BigDecimal(magnitude).setScale(0, RoundingMode.HALF_UP).toPlainString(); } public static byte[] hashUnit(CharSequence unit) { 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 928dc13..4d4a5c9 100644 --- a/core-api/src/test/java/com/absmartly/sdk/ContextTest.java +++ b/core-api/src/test/java/com/absmartly/sdk/ContextTest.java @@ -3173,9 +3173,15 @@ void numericUnitHashesLikeIntegerString() { assertEquals("126", Hashing.unitToString(125.75)); assertEquals("125", Hashing.unitToString(125)); assertEquals("-3", Hashing.unitToString(-2.5)); + assertEquals("-0", Hashing.unitToString(-0.1)); + assertEquals("0", Hashing.unitToString(-0.0)); assertEquals("123456789", Hashing.unitToString(123456789L)); + assertEquals("9007199254740992", Hashing.unitToString(9007199254740993L)); + assertEquals("1e+21", Hashing.unitToString(1e21)); + assertEquals("1.2345e+22", Hashing.unitToString(1.2345e22)); + assertEquals("-1e+21", Hashing.unitToString(-1e21)); + assertEquals("999999999999999868928", Hashing.unitToString(999999999999999900000.0)); - when(eventHandler.publish(any(), any())).thenReturn(CompletableFuture.completedFuture(null)); final Context numeric = createContext(ContextConfig.create().setUnit("user_id", 125.75), dataFutureReady); final Context text = createContext(ContextConfig.create().setUnit("user_id", "126"), dataFutureReady); numeric.track("goal", null); @@ -3221,4 +3227,28 @@ void refreshOnFailedContextIsANoOp() { Mockito.verifyNoInteractions(eventLogger); assertTrue(context.isFailed()); } + + @Test + void setUnitComparesLargeNumericUidsExactly() { + final Context context = createContext(ContextConfig.create().setUnit("user_id", 9007199254740992L), + dataFutureReady); + final IllegalArgumentException error = assertThrows(IllegalArgumentException.class, + () -> context.setUnit("user_id", 9007199254740993L)); + assertEquals("Unit 'user_id' UID already set.", error.getMessage()); + assertDoesNotThrow(() -> context.setUnit("user_id", 9007199254740992.0)); + assertEquals(9007199254740992.0, context.getUnit("user_id")); + } + + @Test + void setUnitRejectsMutableNumbers() { + final Context context = createReadyContext(); + final IllegalArgumentException error = assertThrows(IllegalArgumentException.class, + () -> context.setUnit("account_id", new java.util.concurrent.atomic.AtomicInteger(125))); + assertEquals("Unit 'account_id' must be a string or a number.", error.getMessage()); + assertNull(context.getUnit("account_id")); + + assertThrows(IllegalArgumentException.class, () -> createContext( + ContextConfig.create().setUnit("account_id", new java.util.concurrent.atomic.AtomicLong(1)), + dataFutureReady)); + } } From 80a3c027ff066f41ed464a4f69f474d56888fe68 Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Tue, 27 Oct 2026 14:22:45 +0000 Subject: [PATCH 04/14] fix: JavaScript number formatting for very large unit UIDs; readiness waits respect the Ready event - Use the shortest round-trip digits for numeric UIDs of 1e21 and above, as JavaScript does; Double.toString is not shortest on JDK 11 (1e23 became 9.999999999999999e+22). - waitUntilReady/waitUntilReadyAsync always go through the ready future, so a caller arriving while the Ready event is being logged waits for it. --- .../main/java/com/absmartly/sdk/Context.java | 32 ++++++++----------- .../sdk/internal/hashing/Hashing.java | 17 +++++++++- .../java/com/absmartly/sdk/ContextTest.java | 21 ++++++++++++ 3 files changed, 51 insertions(+), 19 deletions(-) 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 0587294..ea97a46 100644 --- a/core-api/src/main/java/com/absmartly/sdk/Context.java +++ b/core-api/src/main/java/com/absmartly/sdk/Context.java @@ -163,29 +163,25 @@ public boolean isFinalizing() { return isClosing(); } + // Waits go through readyFuture_ rather than data_: data_ is set before the Ready event is logged, + // and the future only completes after it, so a caller never returns before the event. public CompletableFuture waitUntilReadyAsync() { - if (data_ != null) { - return CompletableFuture.completedFuture(this); - } else { - 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); + 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_.get(); // cache here to avoid locking - if (future != null && !future.isDone()) { - future.join(); - } + final CompletableFuture future = readyFuture_.get(); // cache here to avoid locking + if (future != null && !future.isDone()) { + future.join(); } return this; } diff --git a/core-api/src/main/java/com/absmartly/sdk/internal/hashing/Hashing.java b/core-api/src/main/java/com/absmartly/sdk/internal/hashing/Hashing.java index 6af815d..3001dce 100644 --- a/core-api/src/main/java/com/absmartly/sdk/internal/hashing/Hashing.java +++ b/core-api/src/main/java/com/absmartly/sdk/internal/hashing/Hashing.java @@ -1,6 +1,7 @@ package com.absmartly.sdk.internal.hashing; import java.math.BigDecimal; +import java.math.MathContext; import java.math.RoundingMode; import com.absmartly.sdk.internal.Buffers; @@ -37,7 +38,7 @@ public static String unitToString(final Object uid) { final double magnitude = Math.abs(value); final String sign = value < 0 ? "-" : ""; if (magnitude >= 1e21) { - final BigDecimal shortest = new BigDecimal(Double.toString(magnitude)).stripTrailingZeros(); + final BigDecimal shortest = shortestRoundTrip(magnitude).stripTrailingZeros(); final String digits = shortest.unscaledValue().toString(); final int exponent = digits.length() - 1 - shortest.scale(); final String mantissa = digits.length() == 1 ? digits : digits.charAt(0) + "." + digits.substring(1); @@ -46,6 +47,20 @@ public static String unitToString(final Object uid) { return sign + new BigDecimal(magnitude).setScale(0, RoundingMode.HALF_UP).toPlainString(); } + // The fewest significant digits that read back as the same double, nearest to the exact value, as + // JavaScript's Number-to-String conversion picks them. Double.toString is not shortest on every JDK + // (JDK 11 gives 9.999999999999999E22 for 1e23). + private static BigDecimal shortestRoundTrip(final double value) { + final BigDecimal exact = new BigDecimal(value); + for (int precision = 1; precision < 17; ++precision) { + final BigDecimal candidate = exact.round(new MathContext(precision, RoundingMode.HALF_EVEN)); + if (candidate.doubleValue() == value) { + return candidate; + } + } + return exact.round(new MathContext(17, RoundingMode.HALF_EVEN)); + } + public static byte[] hashUnit(CharSequence unit) { final int n = unit.length(); // Up to 4 UTF-8 bytes per UTF-16 code unit (3-byte BMP chars, and 4-byte 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 4d4a5c9..5418c51 100644 --- a/core-api/src/test/java/com/absmartly/sdk/ContextTest.java +++ b/core-api/src/test/java/com/absmartly/sdk/ContextTest.java @@ -3178,6 +3178,8 @@ void numericUnitHashesLikeIntegerString() { assertEquals("123456789", Hashing.unitToString(123456789L)); assertEquals("9007199254740992", Hashing.unitToString(9007199254740993L)); assertEquals("1e+21", Hashing.unitToString(1e21)); + assertEquals("1e+23", Hashing.unitToString(1e23)); + assertEquals("1.7976931348623157e+308", Hashing.unitToString(Double.MAX_VALUE)); assertEquals("1.2345e+22", Hashing.unitToString(1.2345e22)); assertEquals("-1e+21", Hashing.unitToString(-1e21)); assertEquals("999999999999999868928", Hashing.unitToString(999999999999999900000.0)); @@ -3251,4 +3253,23 @@ void setUnitRejectsMutableNumbers() { ContextConfig.create().setUnit("account_id", new java.util.concurrent.atomic.AtomicLong(1)), dataFutureReady)); } + + @Test + void lateReadinessWaitersDoNotSettleBeforeReadyEventIsLogged() { + final AtomicReference lateWaiterDone = new AtomicReference<>(); + final AtomicReference contextRef = new AtomicReference<>(); + Mockito.doAnswer(invocation -> { + if (invocation.getArgument(1) == ContextEventLogger.EventType.Ready) { + lateWaiterDone.set(contextRef.get().waitUntilReadyAsync().isDone()); + } + return null; + }).when(eventLogger).handleEvent(any(), any(), any()); + + final Context context = createContext(dataFuture); + contextRef.set(context); + dataFuture.complete(data); + + assertEquals(Boolean.FALSE, lateWaiterDone.get()); + assertTrue(context.waitUntilReadyAsync().isDone()); + } } From af3501f9dbb829b4f276d55447a59e0526ae26a2 Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Tue, 27 Oct 2026 14:40:08 +0000 Subject: [PATCH 05/14] fix: pick the shortest round-trip digits for large numeric unit UIDs like JavaScript Try both neighbouring decimals at each precision and keep the closest one that reads back as the same double (ECMA-262 Number::toString). The nearest decimal alone misses cases such as 2^89. --- .../sdk/internal/hashing/Hashing.java | 27 +++++++++++++------ .../java/com/absmartly/sdk/ContextTest.java | 5 ++++ 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/core-api/src/main/java/com/absmartly/sdk/internal/hashing/Hashing.java b/core-api/src/main/java/com/absmartly/sdk/internal/hashing/Hashing.java index 3001dce..0229660 100644 --- a/core-api/src/main/java/com/absmartly/sdk/internal/hashing/Hashing.java +++ b/core-api/src/main/java/com/absmartly/sdk/internal/hashing/Hashing.java @@ -47,18 +47,29 @@ public static String unitToString(final Object uid) { return sign + new BigDecimal(magnitude).setScale(0, RoundingMode.HALF_UP).toPlainString(); } - // The fewest significant digits that read back as the same double, nearest to the exact value, as - // JavaScript's Number-to-String conversion picks them. Double.toString is not shortest on every JDK - // (JDK 11 gives 9.999999999999999E22 for 1e23). + // The fewest significant digits that read back as the same double, closest to the exact value when + // two qualify, as in JavaScript's Number-to-String conversion (ECMA-262 Number::toString). Both + // neighbours are tried at each precision: the shortest round-trip decimal is not always the nearest + // one (2^89). Double.toString is not shortest on every JDK (JDK 11 gives 9.999999999999999E22). private static BigDecimal shortestRoundTrip(final double value) { final BigDecimal exact = new BigDecimal(value); - for (int precision = 1; precision < 17; ++precision) { - final BigDecimal candidate = exact.round(new MathContext(precision, RoundingMode.HALF_EVEN)); - if (candidate.doubleValue() == value) { - return candidate; + for (int precision = 1; precision <= 17; ++precision) { + final BigDecimal down = exact.round(new MathContext(precision, RoundingMode.FLOOR)); + final BigDecimal up = exact.round(new MathContext(precision, RoundingMode.CEILING)); + final boolean downOk = down.doubleValue() == value; + final boolean upOk = up.doubleValue() == value; + if (downOk && upOk) { + final int cmp = exact.subtract(down).compareTo(up.subtract(exact)); + return cmp <= 0 ? down : up; + } + if (downOk) { + return down; + } + if (upOk) { + return up; } } - return exact.round(new MathContext(17, RoundingMode.HALF_EVEN)); + return exact; } public static byte[] hashUnit(CharSequence unit) { 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 5418c51..d03f8b1 100644 --- a/core-api/src/test/java/com/absmartly/sdk/ContextTest.java +++ b/core-api/src/test/java/com/absmartly/sdk/ContextTest.java @@ -3179,6 +3179,11 @@ void numericUnitHashesLikeIntegerString() { assertEquals("9007199254740992", Hashing.unitToString(9007199254740993L)); assertEquals("1e+21", Hashing.unitToString(1e21)); assertEquals("1e+23", Hashing.unitToString(1e23)); + assertEquals("6.189700196426902e+26", Hashing.unitToString(Math.scalb(1.0, 89))); + // Expected strings from Node's toFixed(0). + assertEquals("1.1805916207174113e+21", Hashing.unitToString(Math.scalb(1.0, 70))); + assertEquals("3.6267774588438875e+24", Hashing.unitToString(3 * Math.scalb(1.0, 80))); + assertEquals("1.2345678901234569e+23", Hashing.unitToString(123456789012345680000000.0)); assertEquals("1.7976931348623157e+308", Hashing.unitToString(Double.MAX_VALUE)); assertEquals("1.2345e+22", Hashing.unitToString(1.2345e22)); assertEquals("-1e+21", Hashing.unitToString(-1e21)); From b50c6fcba386d13a6dd56b60b5b24f975c8ea4c3 Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Tue, 27 Oct 2026 14:55:33 +0000 Subject: [PATCH 06/14] fix: late close callers wait until the Close event is logged closeAsync() called after the context is marked closed, but while the Close event is still being logged, now returns a future that completes only after the event is logged. --- .../main/java/com/absmartly/sdk/Context.java | 18 +++++++- .../java/com/absmartly/sdk/ContextTest.java | 44 +++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) 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 ea97a46..66a3c17 100644 --- a/core-api/src/main/java/com/absmartly/sdk/Context.java +++ b/core-api/src/main/java/com/absmartly/sdk/Context.java @@ -791,11 +791,14 @@ public CompletableFuture closeAsync() { flush().thenAccept(new Consumer() { @Override public void accept(Void x) { + final CompletableFuture closeLogged = new CompletableFuture(); + closeLoggedFuture_.set(closeLogged); closed_.set(true); closing_.set(false); try { Context.this.logEvent(ContextEventLogger.EventType.Close, null); } finally { + closeLogged.complete(null); newClosingFuture.complete(null); } } @@ -816,10 +819,16 @@ public Void apply(Throwable exception) { return newClosingFuture; } else { + final CompletableFuture closeLogged = new CompletableFuture(); + closeLoggedFuture_.set(closeLogged); closed_.set(true); closing_.set(false); - Context.this.logEvent(ContextEventLogger.EventType.Close, null); + try { + Context.this.logEvent(ContextEventLogger.EventType.Close, null); + } finally { + closeLogged.complete(null); + } // Nothing was pending here, so no closingFuture_ was published for this // attempt; return directly to avoid picking up a stale future left behind @@ -834,6 +843,12 @@ public Void apply(Throwable exception) { } } + // A caller arriving after closed_ is set waits until the Close event has been logged. + final CompletableFuture closeLogged = closeLoggedFuture_.get(); + if (closed_.get() && (closeLogged != null)) { + return closeLogged; + } + return CompletableFuture.completedFuture(null); } @@ -1846,6 +1861,7 @@ private Map buildAttributesMap() { private static final CompletableFuture COMPLETED_VOID_FUTURE = CompletableFuture.completedFuture(null); private final AtomicReference> readyFuture_ = new AtomicReference>(); + private final AtomicReference> closeLoggedFuture_ = new AtomicReference>(); private final AtomicReference> closingFuture_ = new AtomicReference>(); private final AtomicReference> refreshFuture_ = new AtomicReference>(); 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 d03f8b1..66f3394 100644 --- a/core-api/src/test/java/com/absmartly/sdk/ContextTest.java +++ b/core-api/src/test/java/com/absmartly/sdk/ContextTest.java @@ -3277,4 +3277,48 @@ void lateReadinessWaitersDoNotSettleBeforeReadyEventIsLogged() { assertEquals(Boolean.FALSE, lateWaiterDone.get()); assertTrue(context.waitUntilReadyAsync().isDone()); } + + @Test + void lateCloseCallersDoNotSettleBeforeCloseEventIsLogged() { + final AtomicReference lateCloseDone = new AtomicReference<>(); + final AtomicReference contextRef = new AtomicReference<>(); + Mockito.doAnswer(invocation -> { + if (invocation.getArgument(1) == ContextEventLogger.EventType.Close) { + lateCloseDone.set(contextRef.get().closeAsync().isDone()); + } + return null; + }).when(eventLogger).handleEvent(any(), any(), any()); + + final Context context = createReadyContext(); + contextRef.set(context); + context.track("goal1", null); + final CompletableFuture publishFuture = new CompletableFuture<>(); + when(eventHandler.publish(any(), any())).thenReturn(publishFuture); + + final CompletableFuture closing = context.closeAsync(); + publishFuture.complete(null); + closing.join(); + + assertEquals(Boolean.FALSE, lateCloseDone.get()); + assertTrue(context.closeAsync().isDone()); + } + + @Test + void lateCloseCallersWaitForCloseEventWithNothingPending() { + final AtomicReference lateCloseDone = new AtomicReference<>(); + final AtomicReference contextRef = new AtomicReference<>(); + Mockito.doAnswer(invocation -> { + if (invocation.getArgument(1) == ContextEventLogger.EventType.Close) { + lateCloseDone.set(contextRef.get().closeAsync().isDone()); + } + return null; + }).when(eventLogger).handleEvent(any(), any(), any()); + + final Context context = createReadyContext(); + contextRef.set(context); + context.close(); + + assertEquals(Boolean.FALSE, lateCloseDone.get()); + assertTrue(context.closeAsync().isDone()); + } } From 2d464a9af67f7ee9771455669b432712a433a0db Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Tue, 27 Oct 2026 15:14:24 +0000 Subject: [PATCH 07/14] fix: make concurrent closeAsync calls share one attempt that completes after the Close event Starting a close and publishing its future now happen under one lock. Every other caller, whether it arrives while the close is running or after the context is closed, receives that attempt's future, which completes only after the Close event is logged. The Close event is logged once. --- .../main/java/com/absmartly/sdk/Context.java | 99 ++++++++----------- .../java/com/absmartly/sdk/ContextTest.java | 36 +++++++ 2 files changed, 79 insertions(+), 56 deletions(-) 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 66a3c17..dad0671 100644 --- a/core-api/src/main/java/com/absmartly/sdk/Context.java +++ b/core-api/src/main/java/com/absmartly/sdk/Context.java @@ -780,76 +780,63 @@ public void refresh() { } public CompletableFuture closeAsync() { - if (!closed_.get()) { - if (closing_.compareAndSet(false, true)) { - clearRefreshTimer(); - - if (pendingCount_.get() > 0) { - final CompletableFuture newClosingFuture = new CompletableFuture(); - closingFuture_.set(newClosingFuture); + final CompletableFuture attempt; + // The decision to start a close and the publication of its future happen under one lock, so + // every other caller gets that attempt's future, which completes only after the Close event. + synchronized (closeLock_) { + if (closed_.get() || closing_.get()) { + final CompletableFuture current = closingFuture_.get(); + if ((current == null) || (closed_.get() && current.isCompletedExceptionally())) { + return CompletableFuture.completedFuture(null); + } + return current; + } - flush().thenAccept(new Consumer() { - @Override - public void accept(Void x) { - final CompletableFuture closeLogged = new CompletableFuture(); - closeLoggedFuture_.set(closeLogged); - closed_.set(true); - closing_.set(false); - try { - Context.this.logEvent(ContextEventLogger.EventType.Close, null); - } finally { - closeLogged.complete(null); - newClosingFuture.complete(null); - } - } - }).exceptionally(new Function() { - @Override - public Void apply(Throwable exception) { - // If events were restored by flush's failure handler, leave the context - // open so a retry of closeAsync() can attempt to publish them. - if (pendingCount_.get() == 0) { - closed_.set(true); - } - closing_.set(false); - newClosingFuture.completeExceptionally(exception); + closing_.set(true); + attempt = new CompletableFuture(); + closingFuture_.set(attempt); + } - return null; - } - }); + clearRefreshTimer(); - return newClosingFuture; - } else { - final CompletableFuture closeLogged = new CompletableFuture(); - closeLoggedFuture_.set(closeLogged); + if (pendingCount_.get() > 0) { + flush().thenAccept(new Consumer() { + @Override + public void accept(Void x) { closed_.set(true); closing_.set(false); - try { Context.this.logEvent(ContextEventLogger.EventType.Close, null); } finally { - closeLogged.complete(null); + attempt.complete(null); } + } + }).exceptionally(new Function() { + @Override + public Void apply(Throwable exception) { + // If events were restored by flush's failure handler, leave the context + // open so a retry of closeAsync() can attempt to publish them. + if (pendingCount_.get() == 0) { + closed_.set(true); + } + closing_.set(false); + attempt.completeExceptionally(exception); - // Nothing was pending here, so no closingFuture_ was published for this - // attempt; return directly to avoid picking up a stale future left behind - // by an earlier failed close attempt. - return CompletableFuture.completedFuture(null); + return null; } - } + }); + } else { + closed_.set(true); + closing_.set(false); - final CompletableFuture future = closingFuture_.get(); - if (future != null) { - return future; + try { + Context.this.logEvent(ContextEventLogger.EventType.Close, null); + } finally { + attempt.complete(null); } } - // A caller arriving after closed_ is set waits until the Close event has been logged. - final CompletableFuture closeLogged = closeLoggedFuture_.get(); - if (closed_.get() && (closeLogged != null)) { - return closeLogged; - } - - return CompletableFuture.completedFuture(null); + return attempt; } @Override @@ -1861,7 +1848,7 @@ private Map buildAttributesMap() { private static final CompletableFuture COMPLETED_VOID_FUTURE = CompletableFuture.completedFuture(null); private final AtomicReference> readyFuture_ = new AtomicReference>(); - private final AtomicReference> closeLoggedFuture_ = new AtomicReference>(); + private final Object closeLock_ = new Object(); private final AtomicReference> closingFuture_ = new AtomicReference>(); private final AtomicReference> refreshFuture_ = new AtomicReference>(); 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 66f3394..94ab375 100644 --- a/core-api/src/test/java/com/absmartly/sdk/ContextTest.java +++ b/core-api/src/test/java/com/absmartly/sdk/ContextTest.java @@ -3321,4 +3321,40 @@ void lateCloseCallersWaitForCloseEventWithNothingPending() { assertEquals(Boolean.FALSE, lateCloseDone.get()); assertTrue(context.closeAsync().isDone()); } + + @Test + @Timeout(60) + void concurrentClosesLogOneCloseEventAndAllWaitForIt() throws Exception { + final ExecutorService pool = Executors.newFixedThreadPool(3); + try { + for (int i = 0; i < 3000; ++i) { + final java.util.concurrent.atomic.AtomicInteger closeEvents = new java.util.concurrent.atomic.AtomicInteger(); + final java.util.concurrent.atomic.AtomicBoolean callbackDone = new java.util.concurrent.atomic.AtomicBoolean(); + final ContextEventLogger logger = (ctx, type, data) -> { + if (type == ContextEventLogger.EventType.Close) { + closeEvents.incrementAndGet(); + Thread.yield(); + callbackDone.set(true); + } + }; + final Context context = Context.create(clock, ContextConfig.create().setUnits(units), scheduler, + dataFutureReady, dataProvider, eventHandler, logger, variableParser, audienceMatcher); + final java.util.concurrent.CyclicBarrier start = new java.util.concurrent.CyclicBarrier(3); + final List> results = new ArrayList<>(); + for (int t = 0; t < 3; ++t) { + results.add(pool.submit(() -> { + start.await(); + context.closeAsync().join(); + return callbackDone.get(); + })); + } + for (final java.util.concurrent.Future result : results) { + assertTrue(result.get(), "closeAsync returned before the Close event was logged"); + } + assertEquals(1, closeEvents.get(), "Close event logged more than once"); + } + } finally { + pool.shutdownNow(); + } + } } From 933002496855cd5ab350030b188149ebb21a1897 Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Tue, 27 Oct 2026 15:43:05 +0000 Subject: [PATCH 08/14] fix: callers cannot cancel or complete the shared close future The future closeAsync() returns is shared by every caller. It now ignores cancel/complete/obtrude from callers, so one caller cancelling it cannot release the others before the Close event is logged. --- .../main/java/com/absmartly/sdk/Context.java | 43 ++++++++++++++++--- .../java/com/absmartly/sdk/ContextTest.java | 30 +++++++++++++ 2 files changed, 68 insertions(+), 5 deletions(-) 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 dad0671..8c7791e 100644 --- a/core-api/src/main/java/com/absmartly/sdk/Context.java +++ b/core-api/src/main/java/com/absmartly/sdk/Context.java @@ -780,7 +780,7 @@ public void refresh() { } public CompletableFuture closeAsync() { - final CompletableFuture attempt; + final CloseAttempt attempt; // The decision to start a close and the publication of its future happen under one lock, so // every other caller gets that attempt's future, which completes only after the Close event. synchronized (closeLock_) { @@ -793,7 +793,7 @@ public CompletableFuture closeAsync() { } closing_.set(true); - attempt = new CompletableFuture(); + attempt = new CloseAttempt(); closingFuture_.set(attempt); } @@ -808,7 +808,7 @@ public void accept(Void x) { try { Context.this.logEvent(ContextEventLogger.EventType.Close, null); } finally { - attempt.complete(null); + attempt.succeed(); } } }).exceptionally(new Function() { @@ -820,7 +820,7 @@ public Void apply(Throwable exception) { closed_.set(true); } closing_.set(false); - attempt.completeExceptionally(exception); + attempt.fail(exception); return null; } @@ -832,13 +832,46 @@ public Void apply(Throwable exception) { try { Context.this.logEvent(ContextEventLogger.EventType.Close, null); } finally { - attempt.complete(null); + attempt.succeed(); } } return attempt; } + // The close attempt is shared by every closeAsync() caller. Callers cannot cancel or complete it: + // that would release the others before the Close event is logged. Only the context settles it. + private static final class CloseAttempt extends CompletableFuture { + void succeed() { + super.complete(null); + } + + void fail(final Throwable error) { + super.completeExceptionally(error); + } + + @Override + public boolean cancel(final boolean mayInterruptIfRunning) { + return false; + } + + @Override + public boolean complete(final Void value) { + return false; + } + + @Override + public boolean completeExceptionally(final Throwable error) { + return false; + } + + @Override + public void obtrudeValue(final Void value) {} + + @Override + public void obtrudeException(final Throwable error) {} + } + @Override public void close() { closeAsync().join(); 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 94ab375..534242f 100644 --- a/core-api/src/test/java/com/absmartly/sdk/ContextTest.java +++ b/core-api/src/test/java/com/absmartly/sdk/ContextTest.java @@ -3357,4 +3357,34 @@ void concurrentClosesLogOneCloseEventAndAllWaitForIt() throws Exception { pool.shutdownNow(); } } + + @Test + void cancellingOneCloseFutureDoesNotReleaseOtherCallersEarly() { + final AtomicReference lateDone = new AtomicReference<>(); + final AtomicReference contextRef = new AtomicReference<>(); + final AtomicReference> first = new AtomicReference<>(); + Mockito.doAnswer(invocation -> { + if (invocation.getArgument(1) == ContextEventLogger.EventType.Close) { + first.get().cancel(false); + lateDone.set(contextRef.get().closeAsync().isDone()); + } + return null; + }).when(eventLogger).handleEvent(any(), any(), any()); + + final Context context = createReadyContext(); + contextRef.set(context); + context.track("goal1", null); + final CompletableFuture publishFuture = new CompletableFuture<>(); + when(eventHandler.publish(any(), any())).thenReturn(publishFuture); + + first.set(context.closeAsync()); + publishFuture.complete(null); + first.get().join(); + + assertEquals(Boolean.FALSE, lateDone.get()); + assertTrue(context.isClosed()); + final CompletableFuture after = context.closeAsync(); + assertTrue(after.isDone()); + assertFalse(after.isCompletedExceptionally()); + } } From 9e21d05d9609a009931e6f3cc9e0ad4e30cab338 Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Tue, 27 Oct 2026 16:19:12 +0000 Subject: [PATCH 09/14] fix: isolate each close caller from the internal completion gate --- .../main/java/com/absmartly/sdk/Context.java | 54 ++++++------------- .../java/com/absmartly/sdk/ContextTest.java | 25 +++++++-- 2 files changed, 37 insertions(+), 42 deletions(-) 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 8c7791e..770f4e5 100644 --- a/core-api/src/main/java/com/absmartly/sdk/Context.java +++ b/core-api/src/main/java/com/absmartly/sdk/Context.java @@ -780,7 +780,7 @@ public void refresh() { } public CompletableFuture closeAsync() { - final CloseAttempt attempt; + final CompletableFuture attempt; // The decision to start a close and the publication of its future happen under one lock, so // every other caller gets that attempt's future, which completes only after the Close event. synchronized (closeLock_) { @@ -789,11 +789,11 @@ public CompletableFuture closeAsync() { if ((current == null) || (closed_.get() && current.isCompletedExceptionally())) { return CompletableFuture.completedFuture(null); } - return current; + return closeCallerFuture(current); } closing_.set(true); - attempt = new CloseAttempt(); + attempt = new CompletableFuture(); closingFuture_.set(attempt); } @@ -808,7 +808,7 @@ public void accept(Void x) { try { Context.this.logEvent(ContextEventLogger.EventType.Close, null); } finally { - attempt.succeed(); + attempt.complete(null); } } }).exceptionally(new Function() { @@ -820,7 +820,7 @@ public Void apply(Throwable exception) { closed_.set(true); } closing_.set(false); - attempt.fail(exception); + attempt.completeExceptionally(exception); return null; } @@ -832,44 +832,22 @@ public Void apply(Throwable exception) { try { Context.this.logEvent(ContextEventLogger.EventType.Close, null); } finally { - attempt.succeed(); + attempt.complete(null); } } - return attempt; + return closeCallerFuture(attempt); } - // The close attempt is shared by every closeAsync() caller. Callers cannot cancel or complete it: - // that would release the others before the Close event is logged. Only the context settles it. - private static final class CloseAttempt extends CompletableFuture { - void succeed() { - super.complete(null); - } - - void fail(final Throwable error) { - super.completeExceptionally(error); - } - - @Override - public boolean cancel(final boolean mayInterruptIfRunning) { - return false; - } - - @Override - public boolean complete(final Void value) { - return false; - } - - @Override - public boolean completeExceptionally(final Throwable error) { - return false; - } - - @Override - public void obtrudeValue(final Void value) {} - - @Override - public void obtrudeException(final Throwable error) {} + // Do not expose the internal completion gate: completing or cancelling a caller's dependent future + // must not release any other close caller before the Close event is logged. + private static CompletableFuture closeCallerFuture(final CompletableFuture attempt) { + return attempt.thenApply(new Function() { + @Override + public Void apply(Void value) { + return null; + } + }); } @Override 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 534242f..f431181 100644 --- a/core-api/src/test/java/com/absmartly/sdk/ContextTest.java +++ b/core-api/src/test/java/com/absmartly/sdk/ContextTest.java @@ -6,6 +6,7 @@ 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.assertNotSame; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -1791,7 +1792,7 @@ void closeAsync() { final CompletableFuture closingFuture = context.closeAsync(); final CompletableFuture closingFutureNext = context.closeAsync(); - assertSame(closingFuture, closingFutureNext); + assertNotSame(closingFuture, closingFutureNext); assertTrue(context.isClosing()); assertFalse(context.isClosed()); @@ -2672,7 +2673,7 @@ void closeDoesNotCallEventHandlerWhenFailed() { } @Test - void closeAsyncReturnsSameFutureWhenCalledTwice() { + void closeAsyncReturnsIndependentFuturesForTheSameAttempt() { final Context context = createReadyContext(); context.track("goal1", mapOf("amount", 125)); @@ -2683,7 +2684,7 @@ void closeAsyncReturnsSameFutureWhenCalledTwice() { final CompletableFuture closeFuture1 = context.closeAsync(); final CompletableFuture closeFuture2 = context.closeAsync(); - assertSame(closeFuture1, closeFuture2); + assertNotSame(closeFuture1, closeFuture2); publishFuture.complete(null); closeFuture1.join(); @@ -3379,7 +3380,7 @@ void cancellingOneCloseFutureDoesNotReleaseOtherCallersEarly() { first.set(context.closeAsync()); publishFuture.complete(null); - first.get().join(); + context.closeAsync().join(); assertEquals(Boolean.FALSE, lateDone.get()); assertTrue(context.isClosed()); @@ -3387,4 +3388,20 @@ void cancellingOneCloseFutureDoesNotReleaseOtherCallersEarly() { assertTrue(after.isDone()); assertFalse(after.isCompletedExceptionally()); } + + @Test + void externallyCompletingCloseFutureDoesNotReleaseOtherCallers() { + final Context context = createReadyContext(); + context.track("goal1", null); + final CompletableFuture publish = new CompletableFuture<>(); + when(eventHandler.publish(any(), any())).thenReturn(publish); + final CompletableFuture first = context.closeAsync(); + final CompletableFuture second = context.closeAsync(); + first.completeAsync(() -> null).join(); + assertFalse(second.isDone()); + assertFalse(context.closeAsync().isDone()); + publish.complete(null); + second.join(); + assertTrue(context.isClosed()); + } } From 173987797ee2857ca46367af70804ac340ad7837 Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Wed, 28 Oct 2026 10:52:02 +0000 Subject: [PATCH 10/14] fix: keep 1.6.3 compatibility for unit getters, lifecycle ordering, custom-field reads and messages - getUnit/getUnits keep String and Map; typed values via getUnitValue/getUnitValues - complete ready/refresh/close futures before logging the matching event, as in 1.6.3 - custom-field reads work while closing and after close - restore the 1.6.3 IllegalStateException messages - rewrite CHANGELOG and README against 1.6.3; bump to 1.7.0 --- CHANGELOG.md | 18 +-- README.md | 10 +- build.gradle | 2 +- .../main/java/com/absmartly/sdk/Context.java | 93 ++++++----- .../java/com/absmartly/sdk/ContextConfig.java | 23 ++- .../com/absmartly/sdk/ContextEventLogger.java | 6 - .../java/com/absmartly/sdk/ContextTest.java | 149 ++++++++++++------ 7 files changed, 177 insertions(+), 124 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 91ace6d..e4007c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,16 +1,14 @@ # Changelog -## 2.0.0 (unreleased) +## 1.7.0 (unreleased) -### BREAKING +### Changed -- Read methods throw `IllegalStateException` when the context is not ready or is finalized/finalizing, matching the JavaScript SDK `_checkReady` (maintainer decision D1). - - Old: `getTreatment`/`peekTreatment` returned `0`, `getVariableValue`/`peekVariableValue` returned the default value, `getVariableKeys` returned an empty map, `getExperiments`/`getCustomFieldKeys` returned an empty array, `getCustomFieldValue`/`getCustomFieldValueType` returned `null`. - - New: they throw `"ABsmartly Context is not yet ready."` before ready, and (except `getExperiments`, which mirrors JS `experiments()`) `"ABsmartly Context is finalizing."` / `"ABsmartly Context is finalized."` while closing / after close. -- Error message casing changed from `ABSmartly Context ...` to `ABsmartly Context ...`, and `"ABSmartly Context is closing."` became `"ABsmartly Context is finalizing."` to match the JavaScript SDK. -- Unit UIDs can be strings or numbers, like the JavaScript SDK. `ContextConfig.setUnit`/`Context.setUnit` accept a `Number`, `getUnit` returns `Object` and `getUnits`/`ContextConfig.getUnits` return `Map`, keeping the type the UID was set with. A numeric UID is hashed as its value rounded to an integer (`125.75` hashes as `"126"`), as JavaScript's `toFixed(0)` does (including `"-0"` and `"1e+21"` forms). Mutable `Number` types such as `AtomicInteger` are rejected. A blank UID is now reported as blank even when the unit type is already set. +- Unit UIDs can be strings or numbers. `ContextConfig.setUnit` and `Context.setUnit` accept a `Number` in addition to a `String`. New `getUnitValue` and `getUnitValues` accessors retain the original numeric type; the existing `getUnit` and `getUnits` accessors remain source- and binary-compatible and return numeric UIDs in their string form. A numeric UID is hashed as its value rounded to an integer (`125.75` hashes as `"126"`), including `"-0"` and exponential forms for large values. Mutable `Number` types such as `AtomicInteger` are rejected. +- `refresh()` on a context whose initial load failed is a successful no-op: it does not fetch and logs no event. -### Changed +### Compatibility -- The `Ready`, `Refresh`, `Error` and `Close` events are logged before the matching future (`waitUntilReadyAsync`, `refreshAsync`, `closeAsync`) completes, as in the JavaScript SDK, so a caller that waits on the future always sees the event. -- `refresh()` on a context whose initial load failed is a successful no-op: it does not fetch and logs no event, as in the JavaScript SDK. +- Read methods retain the `IllegalStateException` lifecycle behavior of the released 1.6.3 SDK. Custom-field reads continue to work after a context is ready, including while it is closing and after it has closed. +- Event loggers may wait on the matching readiness, refresh, or close future from `handleEvent`. Those futures complete before the associated lifecycle event is logged, as in 1.6.3. +- `IllegalStateException` messages retain the 1.6.3 wording: `ABSmartly Context is not yet ready`, `ABSmartly Context is closing`, and `ABSmartly Context is closed`. diff --git a/README.md b/README.md index cc1c8a2..f055977 100644 --- a/README.md +++ b/README.md @@ -19,15 +19,11 @@ The `android.permission.INTERNET` permission is required. To add this permission If you target Android 6.0 or earlier, a few extra steps are outlined below for installation and initialization. -## Migrating to 2.0.0 (BREAKING) +## Upgrading from 1.6.3 -Read methods now throw `IllegalStateException` instead of returning defaults when the context is not ready, or after it is finalized (matching the JavaScript SDK): +The public unit getters remain compatible with 1.6.3: `getUnit` returns a `String` and `getUnits` returns `Map`. Unit UIDs may now also be numeric; numeric values are returned by the new `getUnitValue` and `getUnitValues` accessors, while the original getters return their string form. -- `getTreatment`, `peekTreatment`, `getVariableValue`, `peekVariableValue`, `getVariableKeys`, `getCustomFieldKeys`, `getCustomFieldValue`, `getCustomFieldValueType` throw `"ABsmartly Context is not yet ready."` before ready, `"ABsmartly Context is finalizing."` while closing and `"ABsmartly Context is finalized."` after close. -- `getExperiments` throws `"ABsmartly Context is not yet ready."` before ready; like JS `experiments()`, it still works after finalize. -- All error messages now use the `ABsmartly` casing (was `ABSmartly`), and the closing message is now `"ABsmartly Context is finalizing."` (was `"ABSmartly Context is closing."`). - -Wait for the context before reading, e.g. `context.waitUntilReady()`, and check `context.isReady()` / `context.isFailed()` if you need a fallback. +Context reads retain the 1.6.3 lifecycle behavior. Before ready they throw `"ABSmartly Context is not yet ready"`; while closing and after close they throw `"ABSmartly Context is closing"` and `"ABSmartly Context is closed"`, respectively. Custom-field reads remain available once the context is ready, including while closing and after close. ## Installation diff --git a/build.gradle b/build.gradle index 4a14d46..736614d 100644 --- a/build.gradle +++ b/build.gradle @@ -12,7 +12,7 @@ plugins { ext { - VERSION = "2.0.0" + VERSION = "1.7.0" GROUP_ID = "com.absmartly.sdk" slf4jVersion = "1.7.30" 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 770f4e5..44d1851 100644 --- a/core-api/src/main/java/com/absmartly/sdk/Context.java +++ b/core-api/src/main/java/com/absmartly/sdk/Context.java @@ -65,7 +65,7 @@ private Context(Clock clock, ContextConfig config, ScheduledExecutorService sche units_ = new HashMap(); - final Map units = config.getUnits(); + final Map units = config.getUnitValues(); if (units != null) { setUnits(units); } @@ -107,15 +107,13 @@ public Void apply(Throwable exception) { @Override public void accept(ContextData data) { Context.this.setData(data); - try { - Context.this.logEvent(ContextEventLogger.EventType.Ready, data); - } finally { - final CompletableFuture rf = readyFuture_.getAndSet(COMPLETED_VOID_FUTURE); - if (rf != null) { - rf.complete(null); - } + final CompletableFuture rf = readyFuture_.getAndSet(COMPLETED_VOID_FUTURE); + if (rf != null) { + rf.complete(null); } + Context.this.logEvent(ContextEventLogger.EventType.Ready, data); + if (Context.this.getPendingCount() > 0) { Context.this.setTimeout(); } @@ -124,15 +122,13 @@ public void accept(ContextData data) { @Override public Void apply(Throwable exception) { Context.this.setDataFailed(exception); - try { - Context.this.logError(exception); - } finally { - final CompletableFuture rf = readyFuture_.getAndSet(COMPLETED_VOID_FUTURE); - if (rf != null) { - rf.complete(null); - } + final CompletableFuture rf = readyFuture_.getAndSet(COMPLETED_VOID_FUTURE); + if (rf != null) { + rf.complete(null); } + Context.this.logError(exception); + return null; } }); @@ -163,8 +159,6 @@ public boolean isFinalizing() { return isClosing(); } - // Waits go through readyFuture_ rather than data_: data_ is set before the Ready event is logged, - // and the future only completes after it, so a caller never returns before the event. public CompletableFuture waitUntilReadyAsync() { final CompletableFuture rf = readyFuture_.get(); if (rf != null) { @@ -205,7 +199,7 @@ public String[] getExperiments() { } public String[] getCustomFieldKeys() { - checkReady(true); + checkReady(false); final Set keys = new HashSet(); @@ -226,7 +220,7 @@ public String[] getCustomFieldKeys() { } public Object getCustomFieldValue(@Nonnull final String experimentName, @Nonnull final String key) { - checkReady(true); + checkReady(false); try { dataLock_.readLock().lock(); @@ -244,7 +238,7 @@ public Object getCustomFieldValue(@Nonnull final String experimentName, @Nonnull } public Object getCustomFieldValueType(@Nonnull final String experimentName, @Nonnull final String key) { - checkReady(true); + checkReady(false); try { dataLock_.readLock().lock(); @@ -306,7 +300,12 @@ public void setCustomAssignments(@Nonnull final Map customAssig } } - public Object getUnit(@Nonnull final String unitType) { + public String getUnit(@Nonnull final String unitType) { + final Object uid = getUnitValue(unitType); + return uid != null ? Hashing.unitToString(uid) : null; + } + + public Object getUnitValue(@Nonnull final String unitType) { final ReentrantReadWriteLock.ReadLock readLock = contextLock_.readLock(); try { readLock.lock(); @@ -420,7 +419,16 @@ private void invalidateAssignmentsPinnedWithMissingUnit(final String unitType) { } } - public Map getUnits() { + public Map getUnits() { + final Map values = getUnitValues(); + final Map units = new HashMap(values.size()); + for (final Map.Entry entry : values.entrySet()) { + units.put(entry.getKey(), Hashing.unitToString(entry.getValue())); + } + return units; + } + + public Map getUnitValues() { final ReentrantReadWriteLock.ReadLock readLock = contextLock_.readLock(); try { readLock.lock(); @@ -747,21 +755,17 @@ public CompletableFuture refreshAsync() { public void accept(ContextData data) { Context.this.setData(data); refreshing_.set(false); - try { - Context.this.logEvent(ContextEventLogger.EventType.Refresh, data); - } finally { - newRefreshFuture.complete(null); - } + newRefreshFuture.complete(null); + + Context.this.logEvent(ContextEventLogger.EventType.Refresh, data); } }).exceptionally(new Function() { @Override public Void apply(Throwable exception) { refreshing_.set(false); - try { - Context.this.logError(exception); - } finally { - newRefreshFuture.completeExceptionally(exception); - } + newRefreshFuture.completeExceptionally(exception); + + Context.this.logError(exception); return null; } }); @@ -782,7 +786,7 @@ public void refresh() { public CompletableFuture closeAsync() { final CompletableFuture attempt; // The decision to start a close and the publication of its future happen under one lock, so - // every other caller gets that attempt's future, which completes only after the Close event. + // every other caller gets that attempt's future. synchronized (closeLock_) { if (closed_.get() || closing_.get()) { final CompletableFuture current = closingFuture_.get(); @@ -805,11 +809,9 @@ public CompletableFuture closeAsync() { public void accept(Void x) { closed_.set(true); closing_.set(false); - try { - Context.this.logEvent(ContextEventLogger.EventType.Close, null); - } finally { - attempt.complete(null); - } + attempt.complete(null); + + Context.this.logEvent(ContextEventLogger.EventType.Close, null); } }).exceptionally(new Function() { @Override @@ -828,19 +830,16 @@ public Void apply(Throwable exception) { } else { closed_.set(true); closing_.set(false); + attempt.complete(null); - try { - Context.this.logEvent(ContextEventLogger.EventType.Close, null); - } finally { - attempt.complete(null); - } + Context.this.logEvent(ContextEventLogger.EventType.Close, null); } return closeCallerFuture(attempt); } // Do not expose the internal completion gate: completing or cancelling a caller's dependent future - // must not release any other close caller before the Close event is logged. + // must not release any other close caller. private static CompletableFuture closeCallerFuture(final CompletableFuture attempt) { return attempt.thenApply(new Function() { @Override @@ -1004,15 +1003,15 @@ public void run() { private void checkNotClosed() { if (closed_.get()) { - throw new IllegalStateException("ABsmartly Context is finalized."); + throw new IllegalStateException("ABSmartly Context is closed"); } else if (closing_.get()) { - throw new IllegalStateException("ABsmartly Context is finalizing."); + 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(); } 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 56d1e95..fdc3f72 100644 --- a/core-api/src/main/java/com/absmartly/sdk/ContextConfig.java +++ b/core-api/src/main/java/com/absmartly/sdk/ContextConfig.java @@ -5,6 +5,8 @@ import javax.annotation.Nonnull; +import com.absmartly.sdk.internal.hashing.Hashing; + public class ContextConfig { public static ContextConfig create() { return new ContextConfig(); @@ -27,11 +29,28 @@ public ContextConfig setUnits(@Nonnull final Map units) { return this; } - public Object getUnit(@Nonnull final String unitType) { + public String getUnit(@Nonnull final String unitType) { + final Object uid = getUnitValue(unitType); + return uid != null ? Hashing.unitToString(uid) : null; + } + + public Object getUnitValue(@Nonnull final String unitType) { return units_ != null ? units_.get(unitType) : null; } - public Map getUnits() { + public Map getUnits() { + if (units_ == null) { + return null; + } + + final Map units = new HashMap(units_.size()); + for (final Map.Entry entry : units_.entrySet()) { + units.put(entry.getKey(), Hashing.unitToString(entry.getValue())); + } + return units; + } + + public Map getUnitValues() { return units_; } diff --git a/core-api/src/main/java/com/absmartly/sdk/ContextEventLogger.java b/core-api/src/main/java/com/absmartly/sdk/ContextEventLogger.java index e030ec3..5b98fda 100644 --- a/core-api/src/main/java/com/absmartly/sdk/ContextEventLogger.java +++ b/core-api/src/main/java/com/absmartly/sdk/ContextEventLogger.java @@ -5,11 +5,5 @@ enum EventType { Error, Ready, Refresh, Publish, Exposure, Goal, Close } - /** - * Called synchronously by the context. Ready, Refresh, Close and Error events are delivered - * before the matching future ({@code waitUntilReadyAsync}, {@code refreshAsync}, - * {@code closeAsync}) completes, so an implementation must not wait on that future: it would - * block forever. - */ void handleEvent(Context context, EventType type, Object data); } 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 f431181..fd7c776 100644 --- a/core-api/src/test/java/com/absmartly/sdk/ContextTest.java +++ b/core-api/src/test/java/com/absmartly/sdk/ContextTest.java @@ -368,7 +368,7 @@ void throwsWhenNotReady() { assertFalse(context.isReady()); assertFalse(context.isFailed()); - final String notReadyMessage = "ABsmartly Context is not yet ready."; + final String notReadyMessage = "ABSmartly Context is not yet ready"; assertEquals(notReadyMessage, assertThrows(IllegalStateException.class, context::getData).getMessage()); assertEquals(notReadyMessage, @@ -409,7 +409,7 @@ void throwsWhenClosing() { assertTrue(context.isClosing()); assertFalse(context.isClosed()); - final String closingMessage = "ABsmartly Context is finalizing."; + final String closingMessage = "ABSmartly Context is closing"; assertEquals(closingMessage, assertThrows(IllegalStateException.class, () -> context.setAttribute("attr1", "value1")).getMessage()); assertEquals(closingMessage, @@ -442,15 +442,10 @@ void throwsWhenClosing() { assertThrows(IllegalStateException.class, () -> context.peekVariableValue("banner.border", 17)) .getMessage()); assertEquals(closingMessage, assertThrows(IllegalStateException.class, context::getVariableKeys).getMessage()); - assertEquals(closingMessage, - assertThrows(IllegalStateException.class, context::getCustomFieldKeys).getMessage()); - assertEquals(closingMessage, - assertThrows(IllegalStateException.class, () -> context.getCustomFieldValue("exp_test_ab", "key")) - .getMessage()); - assertEquals(closingMessage, - assertThrows(IllegalStateException.class, () -> context.getCustomFieldValueType("exp_test_ab", "key")) - .getMessage()); - // matches JS experiments(): _checkReady() without the finalized check + // custom field reads have no lifecycle guard after ready, as in 1.6.3 + assertDoesNotThrow(context::getCustomFieldKeys); + assertDoesNotThrow(() -> context.getCustomFieldValue("exp_test_ab", "key")); + assertDoesNotThrow(() -> context.getCustomFieldValueType("exp_test_ab", "key")); assertEquals(data.experiments.length, context.getExperiments().length); } @@ -469,7 +464,7 @@ void throwsWhenClosed() { assertFalse(context.isClosing()); assertTrue(context.isClosed()); - final String closedMessage = "ABsmartly Context is finalized."; + final String closedMessage = "ABSmartly Context is closed"; assertEquals(closedMessage, assertThrows(IllegalStateException.class, () -> context.setAttribute("attr1", "value1")).getMessage()); assertEquals(closedMessage, @@ -502,15 +497,10 @@ void throwsWhenClosed() { assertThrows(IllegalStateException.class, () -> context.peekVariableValue("banner.border", 17)) .getMessage()); assertEquals(closedMessage, assertThrows(IllegalStateException.class, context::getVariableKeys).getMessage()); - assertEquals(closedMessage, - assertThrows(IllegalStateException.class, context::getCustomFieldKeys).getMessage()); - assertEquals(closedMessage, - assertThrows(IllegalStateException.class, () -> context.getCustomFieldValue("exp_test_ab", "key")) - .getMessage()); - assertEquals(closedMessage, - assertThrows(IllegalStateException.class, () -> context.getCustomFieldValueType("exp_test_ab", "key")) - .getMessage()); - // matches JS experiments(): _checkReady() without the finalized check + // custom field reads have no lifecycle guard after ready, as in 1.6.3 + assertDoesNotThrow(context::getCustomFieldKeys); + assertDoesNotThrow(() -> context.getCustomFieldValue("exp_test_ab", "key")); + assertDoesNotThrow(() -> context.getCustomFieldValueType("exp_test_ab", "key")); assertEquals(data.experiments.length, context.getExperiments().length); } @@ -3065,6 +3055,15 @@ void cancellingPublishResultDoesNotDisturbPublishAccounting() { assertEquals(0, context.getPendingCount()); } + // With pending events the Close event is logged from the publish stage, after closeAsync() settles. + private static T awaitValue(final AtomicReference ref) throws InterruptedException { + final long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(5); + while (ref.get() == null && System.nanoTime() < deadline) { + Thread.sleep(5); + } + return ref.get(); + } + private void recordFutureStateOnEvent(final ContextEventLogger.EventType type, final AtomicReference> future, final AtomicReference doneWhenLogged) { Mockito.doAnswer(invocation -> { @@ -3076,7 +3075,7 @@ private void recordFutureStateOnEvent(final ContextEventLogger.EventType type, } @Test - void readyEventIsLoggedBeforeReadyFutureSettles() { + void readyFutureSettlesBeforeReadyEventIsLogged() { final AtomicReference> future = new AtomicReference<>(); final AtomicReference doneWhenLogged = new AtomicReference<>(); recordFutureStateOnEvent(ContextEventLogger.EventType.Ready, future, doneWhenLogged); @@ -3085,12 +3084,12 @@ void readyEventIsLoggedBeforeReadyFutureSettles() { future.set(context.waitUntilReadyAsync()); dataFuture.complete(data); - assertEquals(Boolean.FALSE, doneWhenLogged.get()); + assertEquals(Boolean.TRUE, doneWhenLogged.get()); assertTrue(future.get().isDone()); } @Test - void errorEventIsLoggedBeforeReadyFutureSettlesOnFailedLoad() { + void readyFutureSettlesBeforeErrorEventIsLoggedOnFailedLoad() { final AtomicReference> future = new AtomicReference<>(); final AtomicReference doneWhenLogged = new AtomicReference<>(); recordFutureStateOnEvent(ContextEventLogger.EventType.Error, future, doneWhenLogged); @@ -3099,12 +3098,12 @@ void errorEventIsLoggedBeforeReadyFutureSettlesOnFailedLoad() { future.set(context.waitUntilReadyAsync()); dataFuture.completeExceptionally(new Exception("FAILED")); - assertEquals(Boolean.FALSE, doneWhenLogged.get()); + assertEquals(Boolean.TRUE, doneWhenLogged.get()); assertTrue(context.isFailed()); } @Test - void refreshEventIsLoggedBeforeRefreshFutureSettles() { + void refreshFutureSettlesBeforeRefreshEventIsLogged() { final AtomicReference> future = new AtomicReference<>(); final AtomicReference doneWhenLogged = new AtomicReference<>(); recordFutureStateOnEvent(ContextEventLogger.EventType.Refresh, future, doneWhenLogged); @@ -3114,12 +3113,12 @@ void refreshEventIsLoggedBeforeRefreshFutureSettles() { future.set(context.refreshAsync()); refreshDataFuture.complete(refreshData); - assertEquals(Boolean.FALSE, doneWhenLogged.get()); + assertEquals(Boolean.TRUE, doneWhenLogged.get()); assertTrue(future.get().isDone()); } @Test - void errorEventIsLoggedBeforeRefreshFutureSettlesOnFailure() { + void refreshFutureSettlesBeforeErrorEventIsLoggedOnFailure() { final AtomicReference> future = new AtomicReference<>(); final AtomicReference doneWhenLogged = new AtomicReference<>(); recordFutureStateOnEvent(ContextEventLogger.EventType.Error, future, doneWhenLogged); @@ -3129,12 +3128,12 @@ void errorEventIsLoggedBeforeRefreshFutureSettlesOnFailure() { future.set(context.refreshAsync()); refreshDataFuture.completeExceptionally(new Exception("FAILED")); - assertEquals(Boolean.FALSE, doneWhenLogged.get()); + assertEquals(Boolean.TRUE, doneWhenLogged.get()); assertTrue(future.get().isCompletedExceptionally()); } @Test - void closeEventIsLoggedBeforeCloseFutureSettles() { + void closeFutureSettlesBeforeCloseEventIsLogged() throws InterruptedException { final AtomicReference> future = new AtomicReference<>(); final AtomicReference doneWhenLogged = new AtomicReference<>(); recordFutureStateOnEvent(ContextEventLogger.EventType.Close, future, doneWhenLogged); @@ -3148,7 +3147,7 @@ void closeEventIsLoggedBeforeCloseFutureSettles() { publishFuture.complete(null); future.get().join(); - assertEquals(Boolean.FALSE, doneWhenLogged.get()); + assertEquals(Boolean.TRUE, awaitValue(doneWhenLogged)); assertTrue(context.isClosed()); } @@ -3161,12 +3160,25 @@ void numericUnitsKeepTheirType() { final Context context = createContext(config, dataFutureReady); context.setUnit("org_id", 99L); - assertEquals(125, context.getUnit("user_id")); - assertEquals(125.75, context.getUnit("float_id")); - assertEquals(99L, context.getUnit("org_id")); - assertEquals("abc", context.getUnit("session_id")); + assertEquals(125, context.getUnitValue("user_id")); + assertEquals(125.75, context.getUnitValue("float_id")); + assertEquals(99L, context.getUnitValue("org_id")); + assertEquals("abc", context.getUnitValue("session_id")); assertEquals(mapOf("session_id", "abc", "user_id", 125, "float_id", 125.75, "org_id", 99L), - context.getUnits()); + context.getUnitValues()); + + // the 1.6.3 getters keep their String types and return the string form + assertEquals("125", context.getUnit("user_id")); + assertEquals("126", context.getUnit("float_id")); + assertEquals("99", context.getUnit("org_id")); + assertEquals("abc", context.getUnit("session_id")); + assertNull(context.getUnit("missing")); + final Map strings = context.getUnits(); + assertEquals(mapOf("session_id", "abc", "user_id", "125", "float_id", "126", "org_id", "99"), strings); + assertEquals("125", config.getUnit("user_id")); + assertEquals(125, config.getUnitValue("user_id")); + assertEquals(mapOf("session_id", "abc", "user_id", "125", "float_id", "126"), config.getUnits()); + assertEquals(mapOf("session_id", "abc", "user_id", 125, "float_id", 125.75), config.getUnitValues()); } @Test @@ -3244,7 +3256,7 @@ void setUnitComparesLargeNumericUidsExactly() { () -> context.setUnit("user_id", 9007199254740993L)); assertEquals("Unit 'user_id' UID already set.", error.getMessage()); assertDoesNotThrow(() -> context.setUnit("user_id", 9007199254740992.0)); - assertEquals(9007199254740992.0, context.getUnit("user_id")); + assertEquals(9007199254740992.0, context.getUnitValue("user_id")); } @Test @@ -3261,7 +3273,7 @@ void setUnitRejectsMutableNumbers() { } @Test - void lateReadinessWaitersDoNotSettleBeforeReadyEventIsLogged() { + void lateReadinessWaitersInsideReadyEventSeeSettledFuture() { final AtomicReference lateWaiterDone = new AtomicReference<>(); final AtomicReference contextRef = new AtomicReference<>(); Mockito.doAnswer(invocation -> { @@ -3275,12 +3287,12 @@ void lateReadinessWaitersDoNotSettleBeforeReadyEventIsLogged() { contextRef.set(context); dataFuture.complete(data); - assertEquals(Boolean.FALSE, lateWaiterDone.get()); + assertEquals(Boolean.TRUE, lateWaiterDone.get()); assertTrue(context.waitUntilReadyAsync().isDone()); } @Test - void lateCloseCallersDoNotSettleBeforeCloseEventIsLogged() { + void lateCloseCallersInsideCloseEventSeeSettledFuture() throws InterruptedException { final AtomicReference lateCloseDone = new AtomicReference<>(); final AtomicReference contextRef = new AtomicReference<>(); Mockito.doAnswer(invocation -> { @@ -3300,12 +3312,12 @@ void lateCloseCallersDoNotSettleBeforeCloseEventIsLogged() { publishFuture.complete(null); closing.join(); - assertEquals(Boolean.FALSE, lateCloseDone.get()); + assertEquals(Boolean.TRUE, awaitValue(lateCloseDone)); assertTrue(context.closeAsync().isDone()); } @Test - void lateCloseCallersWaitForCloseEventWithNothingPending() { + void lateCloseCallersInsideCloseEventSeeSettledFutureWithNothingPending() { final AtomicReference lateCloseDone = new AtomicReference<>(); final AtomicReference contextRef = new AtomicReference<>(); Mockito.doAnswer(invocation -> { @@ -3319,23 +3331,20 @@ void lateCloseCallersWaitForCloseEventWithNothingPending() { contextRef.set(context); context.close(); - assertEquals(Boolean.FALSE, lateCloseDone.get()); + assertEquals(Boolean.TRUE, lateCloseDone.get()); assertTrue(context.closeAsync().isDone()); } @Test @Timeout(60) - void concurrentClosesLogOneCloseEventAndAllWaitForIt() throws Exception { + void concurrentClosesLogOneCloseEvent() throws Exception { final ExecutorService pool = Executors.newFixedThreadPool(3); try { for (int i = 0; i < 3000; ++i) { final java.util.concurrent.atomic.AtomicInteger closeEvents = new java.util.concurrent.atomic.AtomicInteger(); - final java.util.concurrent.atomic.AtomicBoolean callbackDone = new java.util.concurrent.atomic.AtomicBoolean(); final ContextEventLogger logger = (ctx, type, data) -> { if (type == ContextEventLogger.EventType.Close) { closeEvents.incrementAndGet(); - Thread.yield(); - callbackDone.set(true); } }; final Context context = Context.create(clock, ContextConfig.create().setUnits(units), scheduler, @@ -3346,11 +3355,11 @@ void concurrentClosesLogOneCloseEventAndAllWaitForIt() throws Exception { results.add(pool.submit(() -> { start.await(); context.closeAsync().join(); - return callbackDone.get(); + return true; })); } for (final java.util.concurrent.Future result : results) { - assertTrue(result.get(), "closeAsync returned before the Close event was logged"); + assertTrue(result.get()); } assertEquals(1, closeEvents.get(), "Close event logged more than once"); } @@ -3360,7 +3369,7 @@ void concurrentClosesLogOneCloseEventAndAllWaitForIt() throws Exception { } @Test - void cancellingOneCloseFutureDoesNotReleaseOtherCallersEarly() { + void cancellingOneCloseFutureDoesNotDisturbOtherCallers() throws InterruptedException { final AtomicReference lateDone = new AtomicReference<>(); final AtomicReference contextRef = new AtomicReference<>(); final AtomicReference> first = new AtomicReference<>(); @@ -3382,7 +3391,7 @@ void cancellingOneCloseFutureDoesNotReleaseOtherCallersEarly() { publishFuture.complete(null); context.closeAsync().join(); - assertEquals(Boolean.FALSE, lateDone.get()); + assertEquals(Boolean.TRUE, awaitValue(lateDone)); assertTrue(context.isClosed()); final CompletableFuture after = context.closeAsync(); assertTrue(after.isDone()); @@ -3404,4 +3413,42 @@ void externallyCompletingCloseFutureDoesNotReleaseOtherCallers() { second.join(); assertTrue(context.isClosed()); } + + @Test + @Timeout(value = 5, unit = TimeUnit.SECONDS) + void eventLoggerMayWaitForReadinessInsideReadyEvent() { + final AtomicReference readyInLogger = new AtomicReference<>(); + final AtomicReference contextRef = new AtomicReference<>(); + Mockito.doAnswer(invocation -> { + if (invocation.getArgument(1) == ContextEventLogger.EventType.Ready) { + contextRef.get().waitUntilReady(); + contextRef.get().waitUntilReadyAsync().join(); + readyInLogger.set(contextRef.get().isReady()); + } + return null; + }).when(eventLogger).handleEvent(any(), any(), any()); + + final Context context = createContext(dataFuture); + contextRef.set(context); + dataFuture.complete(data); + + assertEquals(Boolean.TRUE, readyInLogger.get()); + } + + @Test + void customFieldReadsStillWorkWhileClosingAndAfterClose() { + final Context context = createReadyContext(); + final String[] keys = context.getCustomFieldKeys(); + context.track("goal1", null); + final CompletableFuture publishFuture = new CompletableFuture<>(); + when(eventHandler.publish(any(), any())).thenReturn(publishFuture); + final CompletableFuture closing = context.closeAsync(); + assertTrue(context.isClosing()); + assertArrayEquals(keys, context.getCustomFieldKeys()); + + publishFuture.complete(null); + closing.join(); + assertTrue(context.isClosed()); + assertArrayEquals(keys, context.getCustomFieldKeys()); + } } From a80489fc12ae88204e6bff10580a100ff572b3b0 Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Wed, 28 Oct 2026 11:05:34 +0000 Subject: [PATCH 11/14] docs: note surrogate-pair hashing and failed-publish close in the changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e4007c4..4d2df92 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ - Unit UIDs can be strings or numbers. `ContextConfig.setUnit` and `Context.setUnit` accept a `Number` in addition to a `String`. New `getUnitValue` and `getUnitValues` accessors retain the original numeric type; the existing `getUnit` and `getUnits` accessors remain source- and binary-compatible and return numeric UIDs in their string form. A numeric UID is hashed as its value rounded to an integer (`125.75` hashes as `"126"`), including `"-0"` and exponential forms for large values. Mutable `Number` types such as `AtomicInteger` are rejected. - `refresh()` on a context whose initial load failed is a successful no-op: it does not fetch and logs no event. +### Fixed + +- Unit UIDs containing supplementary characters (emoji and other code points outside the Basic Multilingual Plane) are now hashed as standard UTF-8, matching the other SDKs. 1.6.3 encoded each half of a surrogate pair separately, so such UIDs hash differently and may be assigned a different variant after upgrading. UIDs made only of ASCII or Basic Multilingual Plane characters are unaffected. +- If the publish during `close()` fails and the events are restored, the context stays open so that `close()` can be called again to retry, and the returned future fails. 1.6.3 marked the context closed in that case. + ### Compatibility - Read methods retain the `IllegalStateException` lifecycle behavior of the released 1.6.3 SDK. Custom-field reads continue to work after a context is ready, including while it is closing and after it has closed. From 25d17f5b2d8d8b5cab4d7915710f5e0da8524b19 Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Wed, 28 Oct 2026 12:06:46 +0000 Subject: [PATCH 12/14] refactor: simplify exponent formatting and drop the atomic wrapper around the close future --- core-api/src/main/java/com/absmartly/sdk/Context.java | 7 ++++--- .../java/com/absmartly/sdk/internal/hashing/Hashing.java | 8 +++----- 2 files changed, 7 insertions(+), 8 deletions(-) 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 44d1851..a89bd9f 100644 --- a/core-api/src/main/java/com/absmartly/sdk/Context.java +++ b/core-api/src/main/java/com/absmartly/sdk/Context.java @@ -789,7 +789,7 @@ public CompletableFuture closeAsync() { // every other caller gets that attempt's future. synchronized (closeLock_) { if (closed_.get() || closing_.get()) { - final CompletableFuture current = closingFuture_.get(); + final CompletableFuture current = closingFuture_; if ((current == null) || (closed_.get() && current.isCompletedExceptionally())) { return CompletableFuture.completedFuture(null); } @@ -798,7 +798,7 @@ public CompletableFuture closeAsync() { closing_.set(true); attempt = new CompletableFuture(); - closingFuture_.set(attempt); + closingFuture_ = attempt; } clearRefreshTimer(); @@ -1859,7 +1859,8 @@ private Map buildAttributesMap() { private static final CompletableFuture COMPLETED_VOID_FUTURE = CompletableFuture.completedFuture(null); private final AtomicReference> readyFuture_ = new AtomicReference>(); private final Object closeLock_ = new Object(); - private final AtomicReference> closingFuture_ = new AtomicReference>(); + // Guarded by closeLock_. + private CompletableFuture closingFuture_; private final AtomicReference> refreshFuture_ = new AtomicReference>(); private final ReentrantLock timeoutLock_ = new ReentrantLock(); diff --git a/core-api/src/main/java/com/absmartly/sdk/internal/hashing/Hashing.java b/core-api/src/main/java/com/absmartly/sdk/internal/hashing/Hashing.java index 0229660..77e014f 100644 --- a/core-api/src/main/java/com/absmartly/sdk/internal/hashing/Hashing.java +++ b/core-api/src/main/java/com/absmartly/sdk/internal/hashing/Hashing.java @@ -38,11 +38,9 @@ public static String unitToString(final Object uid) { final double magnitude = Math.abs(value); final String sign = value < 0 ? "-" : ""; if (magnitude >= 1e21) { - final BigDecimal shortest = shortestRoundTrip(magnitude).stripTrailingZeros(); - final String digits = shortest.unscaledValue().toString(); - final int exponent = digits.length() - 1 - shortest.scale(); - final String mantissa = digits.length() == 1 ? digits : digits.charAt(0) + "." + digits.substring(1); - return sign + mantissa + "e+" + exponent; + // At least 1e21 with at most 17 significant digits, so the scale is negative and + // BigDecimal.toString() always uses scientific notation ("1.2345E+22"). + return sign + shortestRoundTrip(magnitude).stripTrailingZeros().toString().replace('E', 'e'); } return sign + new BigDecimal(magnitude).setScale(0, RoundingMode.HALF_UP).toPlainString(); } From 3c5c0666d491a20b0bacf4fb44d6a289701b1032 Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Wed, 28 Oct 2026 12:26:51 +0000 Subject: [PATCH 13/14] fix: read closing_ before closed_ in closeAsync; cancel the close future while publishing in its regression test - a finishing attempt sets closed_ then clears closing_ outside closeLock_, so the old read order could see both false and start a second close (Close logged and events flushed twice) - the cancellation test now cancels one caller while the publish is pending and checks other callers are unaffected; it fails if closeCallerFuture exposes the shared attempt --- .../main/java/com/absmartly/sdk/Context.java | 4 ++- .../java/com/absmartly/sdk/ContextTest.java | 30 +++++++++---------- 2 files changed, 17 insertions(+), 17 deletions(-) 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 a89bd9f..e144cd7 100644 --- a/core-api/src/main/java/com/absmartly/sdk/Context.java +++ b/core-api/src/main/java/com/absmartly/sdk/Context.java @@ -788,7 +788,9 @@ public CompletableFuture closeAsync() { // The decision to start a close and the publication of its future happen under one lock, so // every other caller gets that attempt's future. synchronized (closeLock_) { - if (closed_.get() || closing_.get()) { + // Read closing_ before closed_: a finishing attempt sets closed_ and then clears closing_ + // outside this lock, so the reverse order can see both false and start a second close. + if (closing_.get() || closed_.get()) { final CompletableFuture current = closingFuture_; if ((current == null) || (closed_.get() && current.isCompletedExceptionally())) { return CompletableFuture.completedFuture(null); 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 fd7c776..d34c6d4 100644 --- a/core-api/src/test/java/com/absmartly/sdk/ContextTest.java +++ b/core-api/src/test/java/com/absmartly/sdk/ContextTest.java @@ -3369,29 +3369,27 @@ void concurrentClosesLogOneCloseEvent() throws Exception { } @Test - void cancellingOneCloseFutureDoesNotDisturbOtherCallers() throws InterruptedException { - final AtomicReference lateDone = new AtomicReference<>(); - final AtomicReference contextRef = new AtomicReference<>(); - final AtomicReference> first = new AtomicReference<>(); - Mockito.doAnswer(invocation -> { - if (invocation.getArgument(1) == ContextEventLogger.EventType.Close) { - first.get().cancel(false); - lateDone.set(contextRef.get().closeAsync().isDone()); - } - return null; - }).when(eventLogger).handleEvent(any(), any(), any()); - + void cancellingOneCloseFutureWhilePublishingDoesNotDisturbOtherCallers() { final Context context = createReadyContext(); - contextRef.set(context); context.track("goal1", null); final CompletableFuture publishFuture = new CompletableFuture<>(); when(eventHandler.publish(any(), any())).thenReturn(publishFuture); - first.set(context.closeAsync()); + final CompletableFuture first = context.closeAsync(); + final CompletableFuture second = context.closeAsync(); + assertTrue(first.cancel(false)); + + assertFalse(second.isDone()); + final CompletableFuture third = context.closeAsync(); + assertFalse(third.isDone()); + assertTrue(context.isClosing()); + publishFuture.complete(null); - context.closeAsync().join(); + second.join(); + third.join(); - assertEquals(Boolean.TRUE, awaitValue(lateDone)); + assertFalse(second.isCancelled()); + assertFalse(third.isCancelled()); assertTrue(context.isClosed()); final CompletableFuture after = context.closeAsync(); assertTrue(after.isDone()); From 77e1f4fcbc47b8a053e22df60d8536de176d408e Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Wed, 28 Oct 2026 12:58:45 +0000 Subject: [PATCH 14/14] refactor: share unit string conversion and extract completeClose - ContextConfig.unitString/unitStrings are used by both Context and ContextConfig - putUnit validates into a final local instead of reassigning its parameter - closeAsync's two completion paths share completeClose --- .../main/java/com/absmartly/sdk/Context.java | 44 +++++++++---------- .../java/com/absmartly/sdk/ContextConfig.java | 21 +++++---- 2 files changed, 34 insertions(+), 31 deletions(-) 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 e144cd7..046af46 100644 --- a/core-api/src/main/java/com/absmartly/sdk/Context.java +++ b/core-api/src/main/java/com/absmartly/sdk/Context.java @@ -301,8 +301,7 @@ public void setCustomAssignments(@Nonnull final Map customAssig } public String getUnit(@Nonnull final String unitType) { - final Object uid = getUnitValue(unitType); - return uid != null ? Hashing.unitToString(uid) : null; + return ContextConfig.unitString(getUnitValue(unitType)); } public Object getUnitValue(@Nonnull final String unitType) { @@ -323,15 +322,19 @@ public void setUnit(@Nonnull final String unitType, @Nonnull final Number uid) { putUnit(unitType, uid); } - private void putUnit(final String unitType, Object uid) { + private void putUnit(final String unitType, final Object value) { checkNotClosed(); - if (uid instanceof String) { - uid = ((String) uid).trim(); - if (((String) uid).isEmpty()) { + final Object uid; + if (value instanceof String) { + final String trimmed = ((String) value).trim(); + if (trimmed.isEmpty()) { throw new IllegalArgumentException(String.format("Unit '%s' UID must not be blank.", unitType)); } - } else if (!isImmutableNumber(uid)) { + uid = trimmed; + } else if (isImmutableNumber(value)) { + uid = value; + } else { throw new IllegalArgumentException(String.format("Unit '%s' must be a string or a number.", unitType)); } @@ -420,12 +423,7 @@ private void invalidateAssignmentsPinnedWithMissingUnit(final String unitType) { } public Map getUnits() { - final Map values = getUnitValues(); - final Map units = new HashMap(values.size()); - for (final Map.Entry entry : values.entrySet()) { - units.put(entry.getKey(), Hashing.unitToString(entry.getValue())); - } - return units; + return ContextConfig.unitStrings(getUnitValues()); } public Map getUnitValues() { @@ -809,11 +807,7 @@ public CompletableFuture closeAsync() { flush().thenAccept(new Consumer() { @Override public void accept(Void x) { - closed_.set(true); - closing_.set(false); - attempt.complete(null); - - Context.this.logEvent(ContextEventLogger.EventType.Close, null); + Context.this.completeClose(attempt); } }).exceptionally(new Function() { @Override @@ -830,16 +824,20 @@ public Void apply(Throwable exception) { } }); } else { - closed_.set(true); - closing_.set(false); - attempt.complete(null); - - Context.this.logEvent(ContextEventLogger.EventType.Close, null); + completeClose(attempt); } return closeCallerFuture(attempt); } + private void completeClose(final CompletableFuture attempt) { + closed_.set(true); + closing_.set(false); + attempt.complete(null); + + logEvent(ContextEventLogger.EventType.Close, null); + } + // Do not expose the internal completion gate: completing or cancelling a caller's dependent future // must not release any other close caller. private static CompletableFuture closeCallerFuture(final CompletableFuture attempt) { 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 fdc3f72..03d9c54 100644 --- a/core-api/src/main/java/com/absmartly/sdk/ContextConfig.java +++ b/core-api/src/main/java/com/absmartly/sdk/ContextConfig.java @@ -30,8 +30,7 @@ public ContextConfig setUnits(@Nonnull final Map units) { } public String getUnit(@Nonnull final String unitType) { - final Object uid = getUnitValue(unitType); - return uid != null ? Hashing.unitToString(uid) : null; + return unitString(getUnitValue(unitType)); } public Object getUnitValue(@Nonnull final String unitType) { @@ -39,13 +38,19 @@ public Object getUnitValue(@Nonnull final String unitType) { } public Map getUnits() { - if (units_ == null) { - return null; - } + return units_ != null ? unitStrings(units_) : null; + } + + // Shared with Context: a numeric UID is returned in its canonical string form, the same string that + // is hashed. + static String unitString(final Object uid) { + return uid != null ? Hashing.unitToString(uid) : null; + } - final Map units = new HashMap(units_.size()); - for (final Map.Entry entry : units_.entrySet()) { - units.put(entry.getKey(), Hashing.unitToString(entry.getValue())); + static Map unitStrings(final Map uids) { + final Map units = new HashMap(uids.size()); + for (final Map.Entry entry : uids.entrySet()) { + units.put(entry.getKey(), unitString(entry.getValue())); } return units; }