Release 7.3.3 #481
Merged
Release 7.3.3 #481
Conversation
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
…d to fix a PE vars issue
…me-batch Fix for variables not being fetched right after a profile push
Changelog for 7.3.3
Updated xcode version for workflow
WalkthroughUpdates CI to use Xcode 16.4. Bumps SDK version to 7.3.3 across metadata. Modifies batching in CleverTap.m to merge profile and events into a single events batch and adjusts dequeueing accordingly. Refactors in-app evaluation manager to combined header/cleanup logic aligned with merged batching. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App
participant CleverTap as CleverTap.m
participant Sender as Network Sender
participant InApps as CTInAppEvaluationManager
Note over CleverTap: New merged batching
App->>CleverTap: trigger sendQueues()
CleverTap->>CleverTap: merge _eventsQueue + _profileQueue -> combined batch
CleverTap->>InApps: onBatchHeaderCreationForQueue(Events)
InApps-->>CleverTap: header (evaluated/suppressed IDs combined)
CleverTap->>Sender: send CTQueueTypeEvents (combined batch, header)
alt success
Sender-->>CleverTap: 200 OK
CleverTap->>CleverTap: remove sent items from _eventsQueue and _profileQueue
CleverTap->>InApps: onBatchSent(Events, header)
InApps->>InApps: removeSent*ForCombined(header)
else failure
Sender-->>CleverTap: error
CleverTap->>CleverTap: retain queues unchanged
end
Note over CleverTap: Notifications queue sent separately (unchanged)
sequenceDiagram
autonumber
participant CT as CTInAppEvaluationManager
participant StoreE as Events Store
participant StoreP as Profile Store
Note over CT: Combined header creation (Events path)
CT->>StoreE: collect evaluated/suppressed IDs
CT->>StoreP: collect evaluated/suppressed IDs
CT-->>CT: merge IDs into header
Note over CT: Combined cleanup after Events batch success
CT->>CT: removeSentEvaluatedServerSideInAppIdsForCombined(header)
CT->>StoreE: prune evaluated IDs (bounded)
CT->>StoreP: prune evaluated IDs (bounded)
CT->>CT: removeSentSuppressedClientSideInAppsForCombined(header)
CT->>StoreE: prune suppressed IDs (bounded)
CT->>StoreP: prune suppressed IDs (bounded)
CT->>StoreE: save
CT->>StoreP: save
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🎉 Snyk checks have passed. No issues have been found so far.✅ code/snyk check is complete. No issues have been found. (View Details) |
nishant-clevertap
approved these changes
Sep 11, 2025
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
.github/workflows/update_package_swift.yml (1)
40-40: Prefer setup-xcode action over hardcoded path.
Hardcoding /Applications/Xcode_16.4.app can break when runner images change. Use maxim-lobanov/setup-xcode to pin 16.4 reliably.Apply:
- - name: Install dependencies - run: | - sudo xcode-select --switch /Applications/Xcode_16.4.app - swift --version - echo "✅ Dependencies installed successfully" + - name: Select Xcode 16.4 + uses: maxim-lobanov/setup-xcode@v1 + with: + xcode-version: '16.4' + - name: Verify toolchain + run: | + xcodebuild -version + swift --version + echo "✅ Xcode toolchain selected"CHANGELOG.md (1)
4-8: Polish heading level and grammar (nit).
- Consider H2 (##) after the H1 to satisfy MD001, or disable the rule consistently across the file.
- Capitalization/pluralization tweak.
-### [Version 7.3.3](https://github.com/CleverTap/clevertap-ios-sdk/releases/tag/7.3.3) (September 11, 2025) +## [Version 7.3.3](https://github.com/CleverTap/clevertap-ios-sdk/releases/tag/7.3.3) (September 11, 2025) @@ -- Optimises queuing behaviour of various type of clevertap events in a single batch. +- Optimises queuing behaviour of various types of CleverTap events into a single batch.Note: If you keep H3 for historical consistency, consider suppressing MD001 for this file instead.
CleverTapSDK/CleverTap.m (1)
2265-2271: Redundant count checks before removals (minor).
removeObjectsInArray is a no-op on empty arrays; the count guards can be dropped.- if ([_profileQueue count] > 0) { - [_profileQueue removeObjectsInArray:batch]; - } - if ([_eventsQueue count] > 0) { - [_eventsQueue removeObjectsInArray:batch]; - } + [_profileQueue removeObjectsInArray:batch]; + [_eventsQueue removeObjectsInArray:batch];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/update_package_swift.yml(1 hunks)CHANGELOG.md(1 hunks)CleverTapSDK/CleverTap.m(2 hunks)CleverTapSDK/CleverTapBuildInfo.h(1 hunks)CleverTapSDK/InApps/CTInAppEvaluationManager.m(3 hunks)Package.swift(1 hunks)sdk-version.txt(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/CTLocalDataStore.m:0-0
Timestamp: 2025-04-17T11:28:19.496Z
Learning: In the CleverTap iOS SDK encryption upgrade (PR #401), error handling has been implemented for encryption/decryption operations to handle potential failures when working with PII data.
Learnt from: akashvercetti
PR: CleverTap/clevertap-ios-sdk#472
File: sdk-version.txt:1-1
Timestamp: 2025-07-31T06:40:28.005Z
Learning: In the CleverTap iOS SDK repository, Package.swift is automatically updated via GitHub Action workflow when the xcframework is built and uploaded, so manual updates to xcframework URLs in Package.swift are not needed during version bumps.
📚 Learning: 2025-04-17T10:51:39.897Z
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTCryptHandler.m:0-0
Timestamp: 2025-04-17T10:51:39.897Z
Learning: The file `CleverTapSDK/Encryption/CTCryptHandler.m` mentioned in a review was removed as part of the encryption algorithm upgrade to AES-GCM in PR #401.
Applied to files:
CleverTapSDK/CleverTapBuildInfo.hCleverTapSDK/InApps/CTInAppEvaluationManager.m
📚 Learning: 2025-07-31T06:40:28.005Z
Learnt from: akashvercetti
PR: CleverTap/clevertap-ios-sdk#472
File: sdk-version.txt:1-1
Timestamp: 2025-07-31T06:40:28.005Z
Learning: In the CleverTap iOS SDK repository, Package.swift is automatically updated via GitHub Action workflow when the xcframework is built and uploaded, so manual updates to xcframework URLs in Package.swift are not needed during version bumps.
Applied to files:
Package.swiftCHANGELOG.md
📚 Learning: 2025-04-17T10:28:14.900Z
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTAESCrypt.swift:0-0
Timestamp: 2025-04-17T10:28:14.900Z
Learning: The file `CleverTapSDK/Encryption/CTAESCrypt.swift` mentioned in a review was removed as part of the encryption algorithm upgrade to AES-GCM in PR #401.
Applied to files:
Package.swift
📚 Learning: 2025-04-17T10:34:46.988Z
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTAESCrypt.swift:0-0
Timestamp: 2025-04-17T10:34:46.988Z
Learning: The file `CleverTapSDK/Encryption/CTAESCrypt.swift` previously used CBC mode without integrity verification, but has been completely replaced with `CTAESGCMCrypt.swift` using AES-GCM (authenticated encryption) in PR #401 to properly address data integrity concerns.
Applied to files:
Package.swift
📚 Learning: 2025-04-17T10:28:14.900Z
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTAESCrypt.swift:0-0
Timestamp: 2025-04-17T10:28:14.900Z
Learning: The file `CleverTapSDK/Encryption/CTAESCrypt.swift` mentioned in a review was removed as part of the encryption algorithm upgrade to AES-GCM in PR #401, replaced by `CTAESGCMCrypt.swift` which implements more secure key management using the Keychain.
Applied to files:
Package.swiftCleverTapSDK/InApps/CTInAppEvaluationManager.m
🪛 markdownlint-cli2 (0.17.2)
CHANGELOG.md
4-4: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🔇 Additional comments (6)
sdk-version.txt (1)
1-1: Version bump looks correct and consistent.
Aligned with the 7.3.3 release. No issues.CleverTapSDK/CleverTapBuildInfo.h (1)
1-1: WR_SDK_REVISION matches 7.3.3.
70303 is correct for 7.3.3. LGTM..github/workflows/update_package_swift.yml (1)
40-40: Verify Xcode 16.4 availability on macos-latest.
Workflow hardcodes:sudo xcode-select --switch /Applications/Xcode_16.4.app— confirm that path exists on the macos-latest runner to avoid CI failures.File: .github/workflows/update_package_swift.yml — around line 40.
#!/bin/bash set -euo pipefail echo "Xcodes present:"; ls -1 /Applications | sed -n 's/^Xcode_//p' || true if [ -d /Applications/Xcode_16.4.app ]; then echo "/Applications/Xcode_16.4.app exists" xcodebuild -version || true else echo "/Applications/Xcode_16.4.app NOT found" ls -1 /Applications || true exit 1 fiPackage.swift (2)
33-35: SPM metadata bump looks good.
URL and checksum updated to 7.3.3; targets unchanged. LGTM.
33-35: Checksum verified — no action required.Computed SHA-256: 9a3a8b7d8118e8d8c5b32744e3d979d051c634d3bddf7542089fb7f3dc3de500 (matches Package.swift).
CleverTapSDK/CleverTap.m (1)
2022-2026: Combining profile + events: ordering & queue-type verified.Profiles are added before events via arrayByAddingObjectsFromArray and the combined batch is sent as CTQueueTypeEvents; CTInAppEvaluationManager only runs the "ForCombined" cleanup when queueType == CTQueueTypeEvents, so the client treats combined payloads as an "events" batch for header/cleanup logic (see CleverTapSDK/CleverTap.m — combined send, and CleverTapSDK/InApps/CTInAppEvaluationManager.m — combined cleanup/onBatchHeaderCreationForQueue).
Comment on lines
+247
to
250
| // For combined queues, clean up both events and profile arrays proportionally | ||
| [self removeSentEvaluatedServerSideInAppIdsForCombined:header]; | ||
| [self removeSentSuppressedClientSideInAppsForCombined:header]; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix non-deterministic dequeue: count-based removal can over-remove.
Between header creation and onBatchSent, new items may be appended; removing by “current counts” can drop unsent items. Dequeue by header contents (identity), and snapshot arrays into the header to make it immutable at send-time. See proposed diffs in the removal methods and header creation below.
Additionally, consider guarding header access to avoid OOB in edge cases:
// outside the changed lines, at the start of onBatchSent:
if (batchWithHeader.count == 0 || ![batchWithHeader[0] isKindOfClass:[NSDictionary class]]) { return; }🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="CleverTapSDK/InApps/CTInAppEvaluationManager.m"
echo "Repo root: $(pwd)"
if [ ! -f "$FILE" ]; then
echo "File not found: $FILE"
echo "Searching for files with similar name..."
rg -n --hidden --no-ignore "CTInAppEvaluationManager.m" || true
exit 0
fi
echo "---- Showing file (numbered) ----"
nl -ba "$FILE" | sed -n '1,400p'
echo "---- Searching for relevant symbols across repo ----"
rg -n --hidden --no-ignore -S "removeSentEvaluatedServerSideInAppIdsForCombined|removeSentSuppressedClientSideInAppsForCombined|onBatchSent|batchWithHeader" || true
echo "---- Context for removal methods in this file ----"
rg -n -S "removeSentEvaluatedServerSideInAppIdsForCombined" -C8 "$FILE" || true
rg -n -S "removeSentSuppressedClientSideInAppsForCombined" -C8 "$FILE" || true
echo "---- Context for onBatchSent ----"
rg -n -S "onBatchSent" -C8 "$FILE" || true
echo "---- Search for array manipulations / dequeue logic in file ----"
rg -n -S "removeObjectAtIndex|removeObjectsInRange|subarrayWithRange|count|mutableCopy|snapshot" -C4 "$FILE" || trueLength of output: 105
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="CleverTapSDK/InApps/CTInAppEvaluationManager.m"
echo "Repo root: $(pwd)"
if [ ! -f "$FILE" ]; then
echo "File not found: $FILE"
echo "Searching for files named CTInAppEvaluationManager.m..."
rg -n --hidden --no-ignore "CTInAppEvaluationManager.m" || true
echo "Searching for symbols containing CTInAppEvaluationManager..."
rg -n --hidden --no-ignore "CTInAppEvaluationManager" || true
exit 0
fi
echo "---- Showing file: $FILE (first 400 lines with numbers) ----"
if command -v cat >/dev/null 2>&1; then
cat -n "$FILE" | sed -n '1,400p' || true
else
sed -n '1,400p' "$FILE" || true
fi
echo "---- Searching repo for relevant symbols ----"
rg -n --hidden --no-ignore -S "removeSentEvaluatedServerSideInAppIdsForCombined|removeSentSuppressedClientSideInAppsForCombined|onBatchSent|batchWithHeader" || true
echo "---- Context for removal methods in this file ----"
rg -n -S "removeSentEvaluatedServerSideInAppIdsForCombined" -C8 "$FILE" || true
rg -n -S "removeSentSuppressedClientSideInAppsForCombined" -C8 "$FILE" || true
echo "---- Context for onBatchSent ----"
rg -n -S "onBatchSent" -C8 "$FILE" || true
echo "---- Search for array manipulations / dequeue logic in file ----"
rg -n -S "removeObjectAtIndex|removeObjectsInRange|subarrayWithRange|count|mutableCopy|snapshot|NSMakeRange" -C4 "$FILE" || trueLength of output: 37812
Fix non-deterministic dequeue: remove by identity or snapshot per-queue arrays
Removing N elements from the live arrays using current counts (removeObjectsInRange) can erase items appended after header creation. Make the header immutable per-queue or remove specific IDs by identity.
- In CTInAppEvaluationManager.m — onBatchHeaderCreationForQueue: — snapshot per-queue arrays into the header (separate events vs profile arrays) or include per-queue counts; do not rely only on a single merged array/count.
- In removeSentEvaluatedServerSideInAppIdsForCombined: and removeSentSuppressedClientSideInAppsForCombined: — remove by identity (iterate header IDs and remove the first matching object from the corresponding live array using indexOfObject/removeObjectAtIndex) instead of removeObjectsInRange(0, N).
- Add a guard in onBatchSent: before accessing batchWithHeader[0]:
if (batchWithHeader.count == 0 || ![batchWithHeader[0] isKindOfClass:[NSDictionary class]]) { return; }
- Update CTInAppEvaluationManagerTest.m to cover the race where items are appended between header creation and onBatchSent.
🤖 Prompt for AI Agents
In CleverTapSDK/InApps/CTInAppEvaluationManager.m around lines 247-250, the code
currently removes ranges from live arrays based on merged counts leading to
non-deterministic dequeues when items are appended after header creation; fix by
snapshotting per-queue arrays into the header (store separate events vs profile
arrays or per-queue counts) at onBatchHeaderCreationForQueue:, change
removeSentEvaluatedServerSideInAppIdsForCombined: and
removeSentSuppressedClientSideInAppsForCombined: to iterate header IDs and
remove each matching object from the live arrays by identity (find indexOfObject
and removeObjectAtIndex) instead of using removeObjectsInRange, and add a guard
in onBatchSent: that returns early if batchWithHeader.count == 0 or
batchWithHeader[0] is not an NSDictionary; also add a unit test in
CTInAppEvaluationManagerTest.m that reproduces appending items between header
creation and onBatchSent to validate the fix.
Comment on lines
+262
to
287
| - (void)removeSentEvaluatedServerSideInAppIdsForCombined:(NSDictionary *)header { | ||
| NSArray *inapps_eval = header[CLTAP_INAPP_SS_EVAL_META_KEY]; | ||
| if (inapps_eval && [inapps_eval count] > 0) { | ||
| NSUInteger len = inapps_eval.count > self.evaluatedServerSideInAppIdsForProfile.count ? self.evaluatedServerSideInAppIdsForProfile.count : inapps_eval.count; | ||
| [self.evaluatedServerSideInAppIdsForProfile removeObjectsInRange:NSMakeRange(0, len)]; | ||
| [self saveEvaluatedServerSideInAppIdsForProfile]; | ||
| // Remove from events array first, then profiles | ||
| NSUInteger eventsCount = [self.evaluatedServerSideInAppIds count]; | ||
| NSUInteger profilesCount = [self.evaluatedServerSideInAppIdsForProfile count]; | ||
| NSUInteger totalToRemove = [inapps_eval count]; | ||
|
|
||
| // Remove from events array | ||
| NSUInteger removeFromEvents = MIN(eventsCount, totalToRemove); | ||
| if (removeFromEvents > 0) { | ||
| [self.evaluatedServerSideInAppIds removeObjectsInRange:NSMakeRange(0, removeFromEvents)]; | ||
| [self saveEvaluatedServerSideInAppIds]; | ||
| totalToRemove -= removeFromEvents; | ||
| } | ||
|
|
||
| // Remove remaining from profiles array | ||
| if (totalToRemove > 0) { | ||
| NSUInteger removeFromProfiles = MIN(profilesCount, totalToRemove); | ||
| if (removeFromProfiles > 0) { | ||
| [self.evaluatedServerSideInAppIdsForProfile removeObjectsInRange:NSMakeRange(0, removeFromProfiles)]; | ||
| [self saveEvaluatedServerSideInAppIdsForProfile]; | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Dequeue by identity (not dynamic counts) to avoid dropping unsent IDs.
- (void)removeSentEvaluatedServerSideInAppIdsForCombined:(NSDictionary *)header {
- NSArray *inapps_eval = header[CLTAP_INAPP_SS_EVAL_META_KEY];
- if (inapps_eval && [inapps_eval count] > 0) {
- // Remove from events array first, then profiles
- NSUInteger eventsCount = [self.evaluatedServerSideInAppIds count];
- NSUInteger profilesCount = [self.evaluatedServerSideInAppIdsForProfile count];
- NSUInteger totalToRemove = [inapps_eval count];
-
- // Remove from events array
- NSUInteger removeFromEvents = MIN(eventsCount, totalToRemove);
- if (removeFromEvents > 0) {
- [self.evaluatedServerSideInAppIds removeObjectsInRange:NSMakeRange(0, removeFromEvents)];
- [self saveEvaluatedServerSideInAppIds];
- totalToRemove -= removeFromEvents;
- }
-
- // Remove remaining from profiles array
- if (totalToRemove > 0) {
- NSUInteger removeFromProfiles = MIN(profilesCount, totalToRemove);
- if (removeFromProfiles > 0) {
- [self.evaluatedServerSideInAppIdsForProfile removeObjectsInRange:NSMakeRange(0, removeFromProfiles)];
- [self saveEvaluatedServerSideInAppIdsForProfile];
- }
- }
- }
+ NSArray *inapps_eval = header[CLTAP_INAPP_SS_EVAL_META_KEY];
+ if (![inapps_eval isKindOfClass:[NSArray class]] || inapps_eval.count == 0) return;
+
+ BOOL eventsChanged = NO, profilesChanged = NO;
+ for (id obj in inapps_eval) {
+ NSUInteger idx = [self.evaluatedServerSideInAppIds indexOfObject:obj];
+ if (idx != NSNotFound) {
+ [self.evaluatedServerSideInAppIds removeObjectAtIndex:idx];
+ eventsChanged = YES;
+ continue;
+ }
+ NSUInteger pidx = [self.evaluatedServerSideInAppIdsForProfile indexOfObject:obj];
+ if (pidx != NSNotFound) {
+ [self.evaluatedServerSideInAppIdsForProfile removeObjectAtIndex:pidx];
+ profilesChanged = YES;
+ }
+ }
+ if (eventsChanged) [self.saveEvaluatedServerSideInAppIds self];
+ if (profilesChanged) [self saveEvaluatedServerSideInAppIdsForProfile];
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In CleverTapSDK/InApps/CTInAppEvaluationManager.m around lines 262 to 287, the
method currently drops the first N entries from local evaluated lists based on
the count of header inapps_eval, which can remove the wrong IDs; instead,
iterate the inapps_eval array and remove each specific ID from
evaluatedServerSideInAppIds and evaluatedServerSideInAppIdsForProfile by
identity/equality (not by positional ranges), updating and saving each list only
when an actual removal occurs; ensure you remove from events list first then
profiles, and call saveEvaluatedServerSideInAppIds and
saveEvaluatedServerSideInAppIdsForProfile after modifications.
Comment on lines
+289
to
314
| - (void)removeSentSuppressedClientSideInAppsForCombined:(NSDictionary *)header { | ||
| NSArray *suppressed_inapps = header[CLTAP_INAPP_SUPPRESSED_META_KEY]; | ||
| if (suppressed_inapps && [suppressed_inapps count] > 0) { | ||
| // Remove from events array first, then profiles | ||
| NSUInteger eventsCount = [self.suppressedClientSideInApps count]; | ||
| NSUInteger profilesCount = [self.suppressedClientSideInAppsForProfile count]; | ||
| NSUInteger totalToRemove = [suppressed_inapps count]; | ||
|
|
||
| // Remove from events array | ||
| NSUInteger removeFromEvents = MIN(eventsCount, totalToRemove); | ||
| if (removeFromEvents > 0) { | ||
| [self.suppressedClientSideInApps removeObjectsInRange:NSMakeRange(0, removeFromEvents)]; | ||
| [self saveSuppressedClientSideInApps]; | ||
| totalToRemove -= removeFromEvents; | ||
| } | ||
|
|
||
| // Remove remaining from profiles array | ||
| if (totalToRemove > 0) { | ||
| NSUInteger removeFromProfiles = MIN(profilesCount, totalToRemove); | ||
| if (removeFromProfiles > 0) { | ||
| [self.suppressedClientSideInAppsForProfile removeObjectsInRange:NSMakeRange(0, removeFromProfiles)]; | ||
| [self saveSuppressedClientSideInAppsForProfile]; | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Apply the same identity-based removal for suppressed in-app entries.
- (void)removeSentSuppressedClientSideInAppsForCombined:(NSDictionary *)header {
- NSArray *suppressed_inapps = header[CLTAP_INAPP_SUPPRESSED_META_KEY];
- if (suppressed_inapps && [suppressed_inapps count] > 0) {
- // Remove from events array first, then profiles
- NSUInteger eventsCount = [self.suppressedClientSideInApps count];
- NSUInteger profilesCount = [self.suppressedClientSideInAppsForProfile count];
- NSUInteger totalToRemove = [suppressed_inapps count];
-
- // Remove from events array
- NSUInteger removeFromEvents = MIN(eventsCount, totalToRemove);
- if (removeFromEvents > 0) {
- [self.suppressedClientSideInApps removeObjectsInRange:NSMakeRange(0, removeFromEvents)];
- [self saveSuppressedClientSideInApps];
- totalToRemove -= removeFromEvents;
- }
-
- // Remove remaining from profiles array
- if (totalToRemove > 0) {
- NSUInteger removeFromProfiles = MIN(profilesCount, totalToRemove);
- if (removeFromProfiles > 0) {
- [self.suppressedClientSideInAppsForProfile removeObjectsInRange:NSMakeRange(0, removeFromProfiles)];
- [self saveSuppressedClientSideInAppsForProfile];
- }
- }
- }
+ NSArray *suppressed_inapps = header[CLTAP_INAPP_SUPPRESSED_META_KEY];
+ if (![suppressed_inapps isKindOfClass:[NSArray class]] || suppressed_inapps.count == 0) return;
+
+ BOOL eventsChanged = NO, profilesChanged = NO;
+ for (id obj in suppressed_inapps) {
+ NSUInteger idx = [self.suppressedClientSideInApps indexOfObject:obj];
+ if (idx != NSNotFound) {
+ [self.suppressedClientSideInApps removeObjectAtIndex:idx];
+ eventsChanged = YES;
+ continue;
+ }
+ NSUInteger pidx = [self.suppressedClientSideInAppsForProfile indexOfObject:obj];
+ if (pidx != NSNotFound) {
+ [self.suppressedClientSideInAppsForProfile removeObjectAtIndex:pidx];
+ profilesChanged = YES;
+ }
+ }
+ if (eventsChanged) [self saveSuppressedClientSideInApps];
+ if (profilesChanged) [self saveSuppressedClientSideInAppsForProfile];
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
CleverTapSDK/InApps/CTInAppEvaluationManager.m around lines 289-314: the current
implementation removes a fixed range of suppressed entries by FIFO count; change
it to identity-based removal instead — iterate the suppressed_inapps array and
for each suppressed entry read its in-app identifier (use the existing
identifier key, e.g. CLTAP_INAPP_ID or "id"), then try to find and remove the
matching entry from self.suppressedClientSideInApps (remove the found object and
call saveSuppressedClientSideInApps); if not found there, try to find and remove
it from self.suppressedClientSideInAppsForProfile (remove and call
saveSuppressedClientSideInAppsForProfile); ensure you only remove exact
identifier matches and handle persistence after each array modification.
Comment on lines
396
to
432
| - (BatchHeaderKeyPathValues)onBatchHeaderCreationForQueue:(CTQueueType)queueType { | ||
| // Evaluation is done for events only at the moment, | ||
| // send the evaluated and suppressed ids in that queue header | ||
| if (queueType != CTQueueTypeEvents && queueType != CTQueueTypeProfile) { | ||
| return [NSMutableDictionary new]; | ||
| } | ||
| NSMutableDictionary *header = [NSMutableDictionary new]; | ||
| if (queueType == CTQueueTypeProfile) { | ||
| if ([self.evaluatedServerSideInAppIdsForProfile count] > 0) { | ||
| header[CLTAP_INAPP_SS_EVAL_META_KEY] = self.evaluatedServerSideInAppIdsForProfile; | ||
| } | ||
| if ([self.suppressedClientSideInAppsForProfile count] > 0) { | ||
| header[CLTAP_INAPP_SUPPRESSED_META_KEY] = self.suppressedClientSideInAppsForProfile; | ||
| } | ||
| } | ||
| else { | ||
| if ([self.evaluatedServerSideInAppIds count] > 0) { | ||
| header[CLTAP_INAPP_SS_EVAL_META_KEY] = self.evaluatedServerSideInAppIds; | ||
| } | ||
| if ([self.suppressedClientSideInApps count] > 0) { | ||
| header[CLTAP_INAPP_SUPPRESSED_META_KEY] = self.suppressedClientSideInApps; | ||
| } | ||
| } | ||
| return header; | ||
| // Evaluation is done for events and profiles, | ||
| // send the evaluated and suppressed ids in that queue header | ||
| if (queueType != CTQueueTypeEvents && queueType != CTQueueTypeProfile) { | ||
| return [NSMutableDictionary new]; | ||
| } | ||
|
|
||
| NSMutableDictionary *header = [NSMutableDictionary new]; | ||
|
|
||
| // For combined queues, merge both events and profile arrays | ||
| if (queueType == CTQueueTypeEvents) { | ||
| // Combine evaluated IDs from both events and profiles | ||
| NSMutableArray *combinedEvaluatedIds = [NSMutableArray array]; | ||
| if ([self.evaluatedServerSideInAppIds count] > 0) { | ||
| [combinedEvaluatedIds addObjectsFromArray:self.evaluatedServerSideInAppIds]; | ||
| } | ||
| if ([self.evaluatedServerSideInAppIdsForProfile count] > 0) { | ||
| [combinedEvaluatedIds addObjectsFromArray:self.evaluatedServerSideInAppIdsForProfile]; | ||
| } | ||
| if ([combinedEvaluatedIds count] > 0) { | ||
| header[CLTAP_INAPP_SS_EVAL_META_KEY] = combinedEvaluatedIds; | ||
| } | ||
|
|
||
| // Combine suppressed IDs from both events and profiles | ||
| NSMutableArray *combinedSuppressedIds = [NSMutableArray array]; | ||
| if ([self.suppressedClientSideInApps count] > 0) { | ||
| [combinedSuppressedIds addObjectsFromArray:self.suppressedClientSideInApps]; | ||
| } | ||
| if ([self.suppressedClientSideInAppsForProfile count] > 0) { | ||
| [combinedSuppressedIds addObjectsFromArray:self.suppressedClientSideInAppsForProfile]; | ||
| } | ||
| if ([combinedSuppressedIds count] > 0) { | ||
| header[CLTAP_INAPP_SUPPRESSED_META_KEY] = combinedSuppressedIds; | ||
| } | ||
| } | ||
| return header; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Snapshot header arrays to avoid mutation between build and send.
Insert immutable copies into the header; otherwise later appends mutate the same arrays referenced by the header.
- if ([combinedEvaluatedIds count] > 0) {
- header[CLTAP_INAPP_SS_EVAL_META_KEY] = combinedEvaluatedIds;
- }
+ if (combinedEvaluatedIds.count > 0) {
+ header[CLTAP_INAPP_SS_EVAL_META_KEY] = [combinedEvaluatedIds copy];
+ }
…
- if ([combinedSuppressedIds count] > 0) {
- header[CLTAP_INAPP_SUPPRESSED_META_KEY] = combinedSuppressedIds;
- }
+ if (combinedSuppressedIds.count > 0) {
+ header[CLTAP_INAPP_SUPPRESSED_META_KEY] = [combinedSuppressedIds copy];
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - (BatchHeaderKeyPathValues)onBatchHeaderCreationForQueue:(CTQueueType)queueType { | |
| // Evaluation is done for events only at the moment, | |
| // send the evaluated and suppressed ids in that queue header | |
| if (queueType != CTQueueTypeEvents && queueType != CTQueueTypeProfile) { | |
| return [NSMutableDictionary new]; | |
| } | |
| NSMutableDictionary *header = [NSMutableDictionary new]; | |
| if (queueType == CTQueueTypeProfile) { | |
| if ([self.evaluatedServerSideInAppIdsForProfile count] > 0) { | |
| header[CLTAP_INAPP_SS_EVAL_META_KEY] = self.evaluatedServerSideInAppIdsForProfile; | |
| } | |
| if ([self.suppressedClientSideInAppsForProfile count] > 0) { | |
| header[CLTAP_INAPP_SUPPRESSED_META_KEY] = self.suppressedClientSideInAppsForProfile; | |
| } | |
| } | |
| else { | |
| if ([self.evaluatedServerSideInAppIds count] > 0) { | |
| header[CLTAP_INAPP_SS_EVAL_META_KEY] = self.evaluatedServerSideInAppIds; | |
| } | |
| if ([self.suppressedClientSideInApps count] > 0) { | |
| header[CLTAP_INAPP_SUPPRESSED_META_KEY] = self.suppressedClientSideInApps; | |
| } | |
| } | |
| return header; | |
| // Evaluation is done for events and profiles, | |
| // send the evaluated and suppressed ids in that queue header | |
| if (queueType != CTQueueTypeEvents && queueType != CTQueueTypeProfile) { | |
| return [NSMutableDictionary new]; | |
| } | |
| NSMutableDictionary *header = [NSMutableDictionary new]; | |
| // For combined queues, merge both events and profile arrays | |
| if (queueType == CTQueueTypeEvents) { | |
| // Combine evaluated IDs from both events and profiles | |
| NSMutableArray *combinedEvaluatedIds = [NSMutableArray array]; | |
| if ([self.evaluatedServerSideInAppIds count] > 0) { | |
| [combinedEvaluatedIds addObjectsFromArray:self.evaluatedServerSideInAppIds]; | |
| } | |
| if ([self.evaluatedServerSideInAppIdsForProfile count] > 0) { | |
| [combinedEvaluatedIds addObjectsFromArray:self.evaluatedServerSideInAppIdsForProfile]; | |
| } | |
| if ([combinedEvaluatedIds count] > 0) { | |
| header[CLTAP_INAPP_SS_EVAL_META_KEY] = combinedEvaluatedIds; | |
| } | |
| // Combine suppressed IDs from both events and profiles | |
| NSMutableArray *combinedSuppressedIds = [NSMutableArray array]; | |
| if ([self.suppressedClientSideInApps count] > 0) { | |
| [combinedSuppressedIds addObjectsFromArray:self.suppressedClientSideInApps]; | |
| } | |
| if ([self.suppressedClientSideInAppsForProfile count] > 0) { | |
| [combinedSuppressedIds addObjectsFromArray:self.suppressedClientSideInAppsForProfile]; | |
| } | |
| if ([combinedSuppressedIds count] > 0) { | |
| header[CLTAP_INAPP_SUPPRESSED_META_KEY] = combinedSuppressedIds; | |
| } | |
| } | |
| return header; | |
| } | |
| - (BatchHeaderKeyPathValues)onBatchHeaderCreationForQueue:(CTQueueType)queueType { | |
| // Evaluation is done for events and profiles, | |
| // send the evaluated and suppressed ids in that queue header | |
| if (queueType != CTQueueTypeEvents && queueType != CTQueueTypeProfile) { | |
| return [NSMutableDictionary new]; | |
| } | |
| NSMutableDictionary *header = [NSMutableDictionary new]; | |
| // For combined queues, merge both events and profile arrays | |
| if (queueType == CTQueueTypeEvents) { | |
| // Combine evaluated IDs from both events and profiles | |
| NSMutableArray *combinedEvaluatedIds = [NSMutableArray array]; | |
| if ([self.evaluatedServerSideInAppIds count] > 0) { | |
| [combinedEvaluatedIds addObjectsFromArray:self.evaluatedServerSideInAppIds]; | |
| } | |
| if ([self.evaluatedServerSideInAppIdsForProfile count] > 0) { | |
| [combinedEvaluatedIds addObjectsFromArray:self.evaluatedServerSideInAppIdsForProfile]; | |
| } | |
| if (combinedEvaluatedIds.count > 0) { | |
| header[CLTAP_INAPP_SS_EVAL_META_KEY] = [combinedEvaluatedIds copy]; | |
| } | |
| // Combine suppressed IDs from both events and profiles | |
| NSMutableArray *combinedSuppressedIds = [NSMutableArray array]; | |
| if ([self.suppressedClientSideInApps count] > 0) { | |
| [combinedSuppressedIds addObjectsFromArray:self.suppressedClientSideInApps]; | |
| } | |
| if ([self.suppressedClientSideInAppsForProfile count] > 0) { | |
| [combinedSuppressedIds addObjectsFromArray:self.suppressedClientSideInAppsForProfile]; | |
| } | |
| if (combinedSuppressedIds.count > 0) { | |
| header[CLTAP_INAPP_SUPPRESSED_META_KEY] = [combinedSuppressedIds copy]; | |
| } | |
| } | |
| return header; | |
| } |
🤖 Prompt for AI Agents
In CleverTapSDK/InApps/CTInAppEvaluationManager.m around lines 396 to 432, the
header currently stores mutable arrays which can be mutated later; replace
assignments that put NSMutableArray instances into header with immutable copies
(e.g., use -copy or create NSArray copies) so the header contains immutable
snapshots. Specifically, when setting CLTAP_INAPP_SS_EVAL_META_KEY and
CLTAP_INAPP_SUPPRESSED_META_KEY assign [combinedEvaluatedIds copy] and
[combinedSuppressedIds copy] (or equivalent immutable copies) instead of the
mutable arrays to prevent later mutations from changing the header contents.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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 by CodeRabbit