[SDK-565] Remove the unused encryptionEnforced field from IterableConfig - #1087
Open
franco-zalamena-iterable wants to merge 1 commit into
Open
[SDK-565] Remove the unused encryptionEnforced field from IterableConfig#1087franco-zalamena-iterable wants to merge 1 commit into
franco-zalamena-iterable wants to merge 1 commit into
Conversation
https://iterable.atlassian.net/browse/SDK-565 3.5.5 announced this option as removed and deleted its public setter, but a merge reinstated the field — without the setter — in 3.6.0, where it has been unsettable and unread ever since. Nothing in the SDK reads it, so removing it cannot change behaviour, and no public or protected signature changes. Adds a reflection guard because nothing in the build detects a re-added member. The guard has teeth against a future re-add, not against today's state: no test can fail while the field is merely present, since nothing reads it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📝 Summary
Deletes the
encryptionEnforcedfield fromIterableConfig. It has had no setter and no reader since 3.5.5 — a merge reinstated the field in 3.6.0 without its setter, and it has sat unsettable and unread ever since.🎟️ Jira Ticket: SDK-565
📖 Description
3.5.5 removed this option and its public
Builder.setEncryptionEnforced(boolean)(#849), and the published CHANGELOG says so:Then
022ea9537(the unknown-user merge, which had forked before the removal) reinstated the field, the builder field and the constructor assignment — but not the setter. A field with no way to set it isn't a design, it's a half-resolved merge conflict. Tag scan ofIterableConfig.javaconfirms the shape:3.5.5–3.5.10→ 0 occurrences,3.6.0onward → 3. So that CHANGELOG line has been untrue for four minor versions. Deleting the field makes the shipped record honest again.This is not a public API change.
final boolean encryptionEnforced;had no access modifier, so it was package-private; the builder's copy wasprivate. No setter, no getter, no read anywhere in the SDK,iterableapi-ui, or the sample app. Nothing outside the package could read it, write it, or see it in autocomplete, so removal breaks no caller — source or binary — and there is no deprecation window to honour.Why remove rather than deprecate (the Jira summary says "deprecate"):
@Deprecatedis a message to callers, and a package-private field with no accessors has none — it would warn nobody. The removal was also already announced to customers 20 months ago, so starting a fresh deprecation clock on it doesn't make sense. This means SDK-647, filed as the "removal half" of a deprecate-then-remove sequence, has no Android work left; I'll close or repoint it separately.Storage behaviour is untouched.
setKeychainEncryption(boolean)andsetDecryptionFailureHandler(...)are the live options and are unaffected.🧪 How to test?
There is no teeth check for the deletion itself, and the tests prove nothing about it. Nothing reads the field, so no test can fail while it is merely present — that's the whole finding. Removal is inert by inspection, not by assertion.
What the new
encryptionEnforcedIsNotPartOfTheConfigurationtest does have teeth against is a future re-add, which is the failure mode that actually happened here. Re-adding the three lines this PR deletes fails it:A reflection guard is admittedly a weak instrument. It's here because nothing in the build is a strong one: there's no
metalava/japicmp/binary-compatibility-validatorand no checked-in API snapshot, so any merge can silently reinstate removed API. That gap is worth its own infra ticket; this PR just guards the one symptom.Full
iterableapisuite and checkstyle pass locally.🧾 Changelog
encryptionEnforcedfield fromIterableConfig. No action required. 3.5.5 announced this option as removed and deleted its public setter, but a merge reinstated the field — without the setter — in 3.6.0, where it has sat unsettable and unread ever since. There has been no way to set it and no effect on SDK behaviour since 3.5.5, so no app can be affected. Storage behaviour is unchanged: usesetKeychainEncryption(boolean)to control whether stored user data is encrypted, andsetDecryptionFailureHandler(...)to be notified when the SDK cannot decrypt it.📹 Loom recording if applicable
N/A — no observable behaviour change.
🐞 Github Issues solved
None.
📚 Docs PR if applicable
Covered on the September release branch (
docs/sdk/september-release): a 3.11.0 release-notes bullet plus a "Removed: the unusedencryptionEnforcedoption" subsection under Upgrading to 3.11.0. That branch's PR covers the whole September release rather than this ticket alone.Out of scope
React Native still ships a public, documented, tested
encryptionEnforced(IterableConfig.ts:330, serialized at:485) that neither native module reads —git grep -i encryptionenforced -- android ioson RN master is empty. That's the surface where the "false sense of security" is real, and where removal genuinely would be source-breaking. Needs its own RN ticket; this PR doesn't affect RN, since the bridge already discards the key.