Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SDK-2506 Removed Events Caching #1441

Merged
merged 2 commits into from
Oct 23, 2024

Conversation

NidhiDixit09
Copy link
Collaborator

@NidhiDixit09 NidhiDixit09 commented Oct 18, 2024

Reference

SDK-2506 -- Remove Events Caching
https://branch.atlassian.net/browse/SDK-2506

Summary

This PR removes on disk caching of events as well as related test cases.

NOTE : Encoding Functions (initWithCoder, encodeWithCoder, supportsSecureCoding) are not removed from request classes. They might be used by app developers if they serialize/archive these objects.

Testing Instructions

Prior behavior: SDK maintained a copy of Server Request Queue on disk. On every cold launch, SDK reads request objects from this disk queue and add then tries to send them again.

After: SDK will not read and write any events to disk.

Steps to test -

  1. Try to fail network requests either by turning off network or force quitting app.
  2. Cold launch app again.
  3. Verify previously failed events are not sent again.

cc @BranchMetrics/saas-sdk-devs for visibility.

Copy link
Contributor

@nsingh-branch nsingh-branch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Just one small nit

static NSString * const BRANCH_QUEUE_FILE = @"BNCServerRequestQueue";
static NSTimeInterval const BATCH_WRITE_TIMEOUT = 3.0;


static inline uint64_t BNCNanoSecondsFromTimeInterval(NSTimeInterval interval) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be removed as well? It seems like its not used anywhere else either

@NidhiDixit09 NidhiDixit09 merged commit 3681750 into master Oct 23, 2024
13 of 14 checks passed
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.

2 participants