Skip to content

SBS review fixes: replace stored settings, keep the consent requirement, device ID trigger, store file - #307

Merged
arifBurakDemiray merged 2 commits into
sdk-behavior-settingsfrom
sbs-review-fixes
Oct 22, 2026
Merged

arifBurakDemiray merged 2 commits into
sdk-behavior-settingsfrom
sbs-review-fixes

Conversation

@arifBurakDemiray

@arifBurakDemiray arifBurakDemiray commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

Stacked on #275. Your uncommitted P7 diff (log gathering, connection test tests, changelog) applies cleanly on top of this branch (git apply --check).

What

Fix Why
Replace, don't merge, the stored settings (port of the reference SDK's settings merge fix, 2026-10-19) Each answer was merged into the stored settings, so a setting removed in SDK Manager kept its last server value for good: a removed tracking:false, eb, jte, lkl or rqs stayed in force across restarts. Now each valid answer replaces the stored c, and a missing key resolves to the developer value or SDK default. removeListingFilterKeysFromConfig is gone.
cr never lifts the developer's requirement Cross-SDK rule decided 2026-10-21: resolved = developer requiresConsent OR a cr true. A server, stored or provided cr:false lifted setRequiresConsent(true): everything tracked without consent and onConsentRemoval was ignored.
Journey trigger on a device ID change deviceIdChanged ends timed events after the new ID is set, so a jte trigger (or a full queue) force-sent the previous user's events under the new ID. Those events now go out together under the previous ID, with no content refresh for the new user.
Store file cut short JsonFileStorage.save() truncated and rewrote in place; a process stopped mid-write left a file that made every later Countly.init throw JSONException into the app. Saves now go through a temp file and an atomic move, an invalid store starts empty, and a leftover temp file is deleted at open.

Verification

  • 9 new tests; each fails on 49c7e03 (cr lifted, removed setting kept, events under new_device_id, JSONException out of init).
  • Full :sdk-java suite, JDK 8, isolated user.home: 675/675, twice.
  • End to end against a live Countly server, whose real settings answer is {}: with setRequiresConsent(true) and provided cr:false, feat: SBS #275 sent events from two runs without consent and this branch sent none. A tracking:false the server dropped in its next answer kept tracking off on feat: SBS #275, not here. The journey trigger events went out under the new device ID on feat: SBS #275 and under the previous one here. A cut-short store file made Countly.init throw on feat: SBS #275, not here.
  • Tests that relied on the merge now carry the settings they need in each response (Content, Filters, Limits, Network, ModuleConfiguration).

Left for you

  • updateConfigVariables javadoc still says "merged settings": your P7 diff edits the adjacent line, so changing it here would conflict.
  • Several Gates/Filters tests push responses without networking(false), so networking turns back on mid-test. They pass because sends to the dummy URL fail and keep the files; only the journey trigger test asserted pending triggers, which is fixed here. GatesTests also overlaps your P7 edits, so it is untouched.
  • cr:true from the server still makes the feature facades return null on the next start (D2), so host code like events().recordEvent(...) throws an NPE. That needs a design decision (non-null no-op facades vs documenting it).
  • Deeper device ID fix: end the old user's events and views, save the profile, then switch the ID (the reference order) in ModuleDeviceIdCore. It would also send views().startView end events under the old ID. Larger blast radius, so not here.
  • Changelog lines for P8: "Mitigated an issue where a setting removed from the SDK behavior settings kept its last value instead of going back to the configuration." and "Mitigated an issue where a store file cut short by a stopped process made initialization fail."

🤖 Generated with Claude Code

The store file was truncated and then written in place on every save, and
the SDK saves it at every init and every settings response. A process
stopped mid-write left a file that made every later Countly.init throw a
JSONException into the application.

Saves now write a temporary file next to the store and move it over the
store, atomically where the file system allows. A store that is not valid
JSON is logged and replaced by empty storage on the next save instead of
failing init, and a temporary file left by an interrupted save is deleted
when the storage opens, so it is not taken for other stored SDK data.
Each response was merged into the stored settings, so a setting removed
in SDK Manager kept its last server value for good, across restarts too: a
removed kill switch, filter, journey trigger, limit or rqs stayed in force.
Each valid response now replaces the stored settings, and a setting it
leaves out goes back to the developer configuration or the SDK default, as
the reference implementation does since its settings merge fix. Values the
validation drops resolve the same way.

A cr false from the server, from storage or from the provided settings no
longer lifts the consent requirement the developer set: the resolved
requirement is the developer's value or a cr true, so the settings can add
the requirement but never remove it.

While a device ID change without merge ends the previous device ID's timed
events and view, a journey trigger or a full event queue reached on that
thread no longer sends the queue under the new device ID; those events go
out together under the previous one, and refresh no content zone.

Tests whose later responses relied on the merge now carry the settings
they need in each response.
@arifBurakDemiray
arifBurakDemiray marked this pull request as ready for review October 22, 2026 06:11
@arifBurakDemiray
arifBurakDemiray merged commit a6fe4ba into sdk-behavior-settings Oct 22, 2026
4 of 5 checks passed
@arifBurakDemiray
arifBurakDemiray deleted the sbs-review-fixes branch October 22, 2026 06:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant