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.

Sign up for GitHub

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

Jump to bottom

Add jitter and extra cache for background processes #366

Merged
merged 18 commits into from Oct 9, 2020
Merged

Add jitter and extra cache for background processes #366

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
7a1cd4e
added a separate value for cache duration to be used when the app is …
aboedo Oct 2, 2020
2008fd6
added jitter for cache updates that start with the app backgrounded.
aboedo Oct 2, 2020
249e032
added purchaserInfoLastUpdatedAt to userDefaults
aboedo Oct 5, 2020
ff4af1e
fixed missing param
aboedo Oct 5, 2020
f6ca9b9
fixed some tests
aboedo Oct 5, 2020
6c0baf1
fixed more tests
aboedo Oct 5, 2020
b42704c
fixed another test
aboedo Oct 5, 2020
23660b2
cleaned up the interface for dispatchWithRandomDelay
aboedo Oct 6, 2020
268bb07
added tests to ensure that newly-created device cache instances read …
aboedo Oct 6, 2020
f301e32
added more tests for device cache
aboedo Oct 6, 2020
b0f4a24
even more device cache tests
aboedo Oct 6, 2020
4dfd0ee
added some new test cases to purchasesTests
aboedo Oct 6, 2020
50240f6
added test to ensure we don't automatically fetch purchaser info onDi…
aboedo Oct 6, 2020
9e3a02a
updated maxJitterInSeconds to 5
aboedo Oct 8, 2020
724a491
formatting of * _Nullable
aboedo Oct 8, 2020
8680bce
simplified conditions and improved nil handling in isPurchaserInfoCac…
aboedo Oct 9, 2020
3904ea4
small cleanup
aboedo Oct 9, 2020
15bbf4a
added test case for migration
aboedo Oct 9, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions Purchases/Caching/RCDeviceCache.h
View file
Open in desktop
Expand Up @@ -33,21 +33,21 @@ NS_ASSUME_NONNULL_BEGIN

- (void)cachePurchaserInfo:(NSData *)data forAppUserID:(NSString *)appUserID;

- (BOOL)isPurchaserInfoCacheStale;
- (BOOL)isPurchaserInfoCacheStaleForAppUserID:(NSString *)appUserID isAppBackgrounded:(BOOL)isAppBackgrounded;
Copy link
Member Author

Choose a reason for hiding this comment

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

not a huge fan of how this API is turning out, it feels overly complicated. But I didn't want to perform a major refactor for this.


- (void)clearPurchaserInfoCacheTimestamp;
- (void)clearPurchaserInfoCacheTimestampForAppUserID:(NSString *)appUserID;

- (void)clearPurchaserInfoCacheForAppUserID:(NSString *)appUserID;

- (void)setPurchaserInfoCacheTimestampToNow;
- (void)setPurchaserInfoCacheTimestampToNowForAppUserID:(NSString *)appUserID;

#pragma mark - offerings

@property (nonatomic, readonly, nullable) RCOfferings *cachedOfferings;

- (void)cacheOfferings:(RCOfferings *)offerings;

- (BOOL)isOfferingsCacheStale;
- (BOOL)isOfferingsCacheStaleWithIsAppBackgrounded:(BOOL)isAppBackgrounded;

- (void)clearOfferingsCacheTimestamp;

Expand Down
57 changes: 38 additions & 19 deletions Purchases/Caching/RCDeviceCache.m
View file
Open in desktop
Expand Up @@ -8,15 +8,12 @@

#import "RCDeviceCache.h"
#import "RCDeviceCache+Protected.h"
#import "RCLogUtils.h"
#import "NSDictionary+RCExtensions.h"
Comment on lines -11 to -12
Copy link
Member Author

Choose a reason for hiding this comment

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

unused imports


@interface RCDeviceCache ()

@property (nonatomic) NSUserDefaults *userDefaults;
@property (nonatomic) NSNotificationCenter *notificationCenter;
@property (nonatomic, nonnull) RCInMemoryCachedObject<RCOfferings *> *offeringsCachedObject;
@property (nonatomic, nullable) NSDate *purchaserInfoCachesLastUpdated;
Copy link
Member Author

Choose a reason for hiding this comment

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

We were storing this value in memory before. This doesn't work well for widgets, since the process for the widget will likely be killed before the next refresh, so it'll always refresh cache.


@end

Expand All @@ -26,11 +23,12 @@ @interface RCDeviceCache ()
NSString *RCLegacyGeneratedAppUserDefaultsKey = RC_CACHE_KEY_PREFIX @".appUserID";
NSString *RCAppUserDefaultsKey = RC_CACHE_KEY_PREFIX @".appUserID.new";
NSString *RCPurchaserInfoAppUserDefaultsKeyBase = RC_CACHE_KEY_PREFIX @".purchaserInfo.";
NSString *RCPurchaserInfoLastUpdatedKeyBase = RC_CACHE_KEY_PREFIX @".purchaserInfoLastUpdated.";
Copy link
Member Author

@aboedo aboedo Oct 5, 2020 •
edited

Choose a reason for hiding this comment

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

new userDefaults entry

NSString *RCLegacySubscriberAttributesKeyBase = RC_CACHE_KEY_PREFIX @".subscriberAttributes.";
NSString *RCSubscriberAttributesKey = RC_CACHE_KEY_PREFIX @".subscriberAttributes";
NSString *RCAttributionDataDefaultsKeyBase = RC_CACHE_KEY_PREFIX @".attribution.";
#define CACHE_DURATION_IN_SECONDS 60 * 5

int cacheDurationInSecondsInForeground = 60 * 5;
int cacheDurationInSecondsInBackground = 60 * 60 * 24;

@implementation RCDeviceCache

Expand All @@ -44,8 +42,7 @@ - (instancetype)initWith:(NSUserDefaults *)userDefaults
self = [super init];
if (self) {
if (offeringsCachedObject == nil) {
offeringsCachedObject =
[[RCInMemoryCachedObject alloc] initWithCacheDurationInSeconds:CACHE_DURATION_IN_SECONDS];
offeringsCachedObject = [[RCInMemoryCachedObject alloc] init];
}
self.offeringsCachedObject = offeringsCachedObject;

Expand Down Expand Up @@ -106,7 +103,7 @@ - (void)clearCachesForAppUserID:(NSString *)oldAppUserID andSaveNewUserID:(NSStr
@synchronized (self) {
[self.userDefaults removeObjectForKey:RCLegacyGeneratedAppUserDefaultsKey];
[self.userDefaults removeObjectForKey:[self purchaserInfoUserDefaultCacheKeyForAppUserID:oldAppUserID]];
[self clearPurchaserInfoCacheTimestamp];
[self clearPurchaserInfoCacheTimestampForAppUserID:oldAppUserID];
[self clearOfferingsCache];

[self deleteAttributesIfSyncedForAppUserID:oldAppUserID];
Expand All @@ -125,34 +122,55 @@ - (void)cachePurchaserInfo:(NSData *)data forAppUserID:(NSString *)appUserID {
@synchronized (self) {
[self.userDefaults setObject:data
forKey:[self purchaserInfoUserDefaultCacheKeyForAppUserID:appUserID]];
[self setPurchaserInfoCacheTimestampToNow];
[self setPurchaserInfoCacheTimestampToNowForAppUserID:appUserID];
Copy link
Member Author

Choose a reason for hiding this comment

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

this didn't used to be tied to an appUserID, but I tied it now for the sake of consistency.

}
}

- (BOOL)isPurchaserInfoCacheStale {
NSTimeInterval timeSinceLastCheck = -[self.purchaserInfoCachesLastUpdated timeIntervalSinceNow];
return !(self.purchaserInfoCachesLastUpdated != nil && timeSinceLastCheck < CACHE_DURATION_IN_SECONDS);
- (BOOL)isPurchaserInfoCacheStaleForAppUserID:(NSString *)appUserID isAppBackgrounded:(BOOL)isAppBackgrounded {
NSDate * _Nullable purchaserInfoCachesLastUpdated = [self purchaserInfoCachesLastUpdatedForAppUserID:appUserID];
NSTimeInterval timeSinceLastCheck = -[purchaserInfoCachesLastUpdated timeIntervalSinceNow];
int cacheDurationInSeconds = [self cacheDurationInSecondsWithIsAppBackgrounded:isAppBackgrounded];
return !(purchaserInfoCachesLastUpdated != nil && timeSinceLastCheck < cacheDurationInSeconds);
}

- (void)clearPurchaserInfoCacheTimestamp {
self.purchaserInfoCachesLastUpdated = nil;
- (int)cacheDurationInSecondsWithIsAppBackgrounded:(BOOL)isAppBackgrounded {
return (isAppBackgrounded ? cacheDurationInSecondsInBackground
: cacheDurationInSecondsInForeground);
}

- (void)clearPurchaserInfoCacheTimestampForAppUserID:(NSString *)appUserID {
NSString *cacheKey = [self purchaserInfoLastUpdatedCacheKeyForAppUserID:appUserID];
[self.userDefaults removeObjectForKey:cacheKey];
}

- (void)clearPurchaserInfoCacheForAppUserID:(NSString *)appUserID {
@synchronized (self) {
[self clearPurchaserInfoCacheTimestamp];
[self clearPurchaserInfoCacheTimestampForAppUserID:appUserID];
[self.userDefaults removeObjectForKey:[self purchaserInfoUserDefaultCacheKeyForAppUserID:appUserID]];
}
}

- (void)setPurchaserInfoCacheTimestampToNow {
self.purchaserInfoCachesLastUpdated = [NSDate date];
- (void)setPurchaserInfoCacheTimestampToNowForAppUserID:(NSString *)appUserID {
[self setPurchaserInfoCacheTimestamp:[NSDate date] forAppUserID:appUserID];
}

- (void)setPurchaserInfoCacheTimestamp:(NSDate *)timestamp forAppUserID:(NSString *)appUserID {
NSString *cacheKey = [self purchaserInfoLastUpdatedCacheKeyForAppUserID:appUserID];
[self.userDefaults setObject:timestamp forKey:cacheKey];
}

- (nullable NSDate *)purchaserInfoCachesLastUpdatedForAppUserID:(NSString *)appUserID {
return (NSDate*)[self.userDefaults objectForKey:[self purchaserInfoLastUpdatedCacheKeyForAppUserID:appUserID]];
}

- (NSString *)purchaserInfoUserDefaultCacheKeyForAppUserID:(NSString *)appUserID {
return [RCPurchaserInfoAppUserDefaultsKeyBase stringByAppendingString:appUserID];
}

- (NSString *)purchaserInfoLastUpdatedCacheKeyForAppUserID:(NSString *)appUserID {
return [RCPurchaserInfoLastUpdatedKeyBase stringByAppendingString:appUserID];
}

#pragma mark - offerings

- (nullable RCOfferings *)cachedOfferings {
Expand All @@ -163,8 +181,9 @@ - (void)cacheOfferings:(RCOfferings *)offerings {
[self.offeringsCachedObject cacheInstance:offerings];
}

- (BOOL)isOfferingsCacheStale {
return self.offeringsCachedObject.isCacheStale;
- (BOOL)isOfferingsCacheStaleWithIsAppBackgrounded:(BOOL)isAppBackgrounded {
Copy link
Member Author

Choose a reason for hiding this comment

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

having a different cache might not be strictly necessary for offerings, really - it's unlikely that you'd need offerings while the app is running in the background. can't hurt, though.

int cacheDurationInSeconds = [self cacheDurationInSecondsWithIsAppBackgrounded:isAppBackgrounded];
return [self.offeringsCachedObject isCacheStaleWithDurationInSeconds:cacheDurationInSeconds];
}

- (void)clearOfferingsCacheTimestamp {
Expand Down
6 changes: 1 addition & 5 deletions Purchases/Caching/RCInMemoryCachedObject.h
View file
Open in desktop
Expand Up @@ -10,7 +10,7 @@ NS_ASSUME_NONNULL_BEGIN

@interface RCInMemoryCachedObject<ObjectType: id<NSObject>> : NSObject

- (BOOL)isCacheStale;
- (BOOL)isCacheStaleWithDurationInSeconds:(int)durationInSeconds;

- (void)clearCacheTimestamp;

Expand All @@ -22,10 +22,6 @@ NS_ASSUME_NONNULL_BEGIN

- (nullable ObjectType)cachedInstance;

- (instancetype)init NS_UNAVAILABLE;

- (instancetype)initWithCacheDurationInSeconds:(int)cacheDurationInSeconds;

@end


Expand Down
12 changes: 2 additions & 10 deletions Purchases/Caching/RCInMemoryCachedObject.m
View file
Open in desktop
Expand Up @@ -16,28 +16,20 @@
@interface RCInMemoryCachedObject ()

@property (nonatomic, nullable) NSDate *lastUpdatedAt;
@property (nonatomic, assign) int cacheDurationInSeconds;
@property (nonatomic, nullable) id cachedInstance;

@end


@implementation RCInMemoryCachedObject

- (instancetype)initWithCacheDurationInSeconds:(int)cacheDurationInSeconds {
if (self = [super init]) {
self.cacheDurationInSeconds = cacheDurationInSeconds;
}
return self;
}

- (BOOL)isCacheStale {
- (BOOL)isCacheStaleWithDurationInSeconds:(int)durationInSeconds {
if (self.lastUpdatedAt == nil) {
return YES;
}

NSTimeInterval timeSinceLastCheck = -1 * [self.lastUpdatedAt timeIntervalSinceNow];
return timeSinceLastCheck >= self.cacheDurationInSeconds;
return timeSinceLastCheck >= durationInSeconds;
}

- (void)clearCacheTimestamp {
Expand Down
2 changes: 1 addition & 1 deletion Purchases/ProtectedExtensions/RCDeviceCache+Protected.h
View file
Open in desktop
Expand Up @@ -13,7 +13,7 @@ NS_ASSUME_NONNULL_BEGIN

@interface RCDeviceCache (Protected)

@property (nonatomic, nullable) NSDate *purchaserInfoCachesLastUpdated;
- (void)setPurchaserInfoCacheTimestamp:(NSDate *)timestamp forAppUserID:(NSString *)appUserID;

- (nullable instancetype)initWith:(nullable NSUserDefaults *)userDefaults
offeringsCachedObject:(nullable RCInMemoryCachedObject<RCOfferings *> *)offeringsCachedObject
Expand Down
131 changes: 72 additions & 59 deletions Purchases/Public/RCPurchases.m
View file
Open in desktop
Expand Up @@ -299,7 +299,7 @@ - (instancetype)initWithAppUserID:(nullable NSString *)appUserID

[self.systemInfo isApplicationBackgroundedWithCompletion:^(BOOL isBackgrounded) {
if (!isBackgrounded) {
[self.operationDispatcher dispatchOnWorkerThread:^{
[self.operationDispatcher dispatchOnWorkerThreadWithRandomDelay:NO block:^{
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about keeping around dispatchOnWorkerThread in operationDispatcher and make it call dispatchOnWorkerThreadWithRandomDelay:NO. I like it more since most of the times we will be passing NO.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, I'll separate into two methods. I'd started by doing that, then I moved to having the randomDelay be optional, then the optional randomDelay looked great in swift but ported very poorly to obj-c, so I killed the optionality. I'll go back to the start.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, I was trying this and remembered why I ended up going with the param: it's so that callers don't have to choose between methods, since almost every time they pass in the value of isBackgrounded.

So if there are two separate methods, you'd have to do if isBackgrounded dispatchWithRandomDelay else dispatch.
note that we almost every time pass isBackgrounded. even in this line, we're passing NO because we know that isBackgrounded is false, because of the if in the line right above. But we could just as easily pass isBackgrounded instead.

[self updateAllCachesWithCompletionBlock:callDelegate];
}];
} else {
Expand Down Expand Up @@ -421,18 +421,20 @@ - (void)resetWithCompletionBlock:(nullable RCReceivePurchaserInfoBlock)completio
}

- (void)purchaserInfoWithCompletionBlock:(RCReceivePurchaserInfoBlock)completion {
RCPurchaserInfo *infoFromCache = [self readPurchaserInfoFromCache];
if (infoFromCache) {
RCDebugLog(@"Vending purchaserInfo from cache");
CALL_IF_SET_ON_MAIN_THREAD(completion, infoFromCache, nil);
if ([self.deviceCache isPurchaserInfoCacheStale]) {
RCDebugLog(@"Cache is stale, updating caches");
[self fetchAndCachePurchaserInfoWithCompletion:nil];
[self.systemInfo isApplicationBackgroundedWithCompletion:^(BOOL isAppBackgrounded) {
RCPurchaserInfo *infoFromCache = [self readPurchaserInfoFromCache];
if (infoFromCache) {
RCDebugLog(@"Vending purchaserInfo from cache");
CALL_IF_SET_ON_MAIN_THREAD(completion, infoFromCache, nil);
if ([self.deviceCache isPurchaserInfoCacheStaleForAppUserID:self.appUserID isAppBackgrounded:isAppBackgrounded]) {
RCDebugLog(@"Cache is stale, updating caches");
[self fetchAndCachePurchaserInfoWithCompletion:nil isAppBackgrounded:isAppBackgrounded];
}
} else {
RCDebugLog(@"No cached purchaser info, fetching");
[self fetchAndCachePurchaserInfoWithCompletion:completion isAppBackgrounded:isAppBackgrounded];
Comment on lines +424 to +435
Copy link
Member Author

Choose a reason for hiding this comment

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

really don't love making these even more complex, but I couldn't think of a way around it given that the isAppBackgrounded method is async.

}
} else {
RCDebugLog(@"No cached purchaser info, fetching");
[self fetchAndCachePurchaserInfoWithCompletion:completion];
}
}];
}

#pragma mark Purchasing
Expand Down Expand Up @@ -534,11 +536,12 @@ - (void) purchaseProduct:(SKProduct *)product
if (!self.finishTransactions) {
RCDebugLog(@"makePurchase - Observer mode is active (finishTransactions is set to false) and makePurchase has been called. Are you sure you want to do this?");
}
payment.applicationUsername = self.appUserID;
NSString *appUserID = self.appUserID;
payment.applicationUsername = appUserID;

// This is to prevent the UIApplicationDidBecomeActive call from the purchase popup
// from triggering a refresh.
[self.deviceCache setPurchaserInfoCacheTimestampToNow];
[self.deviceCache setPurchaserInfoCacheTimestampToNowForAppUserID:appUserID];
[self.deviceCache setOfferingsCacheTimestampToNow];

if (presentedOfferingIdentifier) {
Expand Down Expand Up @@ -821,14 +824,16 @@ - (void)sendCachedPurchaserInfoIfAvailable {

- (void)updateAllCachesIfNeeded {
RCDebugLog(@"applicationDidBecomeActive");
if ([self.deviceCache isPurchaserInfoCacheStale]) {
RCDebugLog(@"PurchaserInfo cache is stale, updating caches");
[self fetchAndCachePurchaserInfoWithCompletion:nil];
}
if ([self.deviceCache isOfferingsCacheStale]) {
RCDebugLog(@"Offerings cache is stale, updating caches");
[self updateOfferingsCache:nil];
}
[self.systemInfo isApplicationBackgroundedWithCompletion:^(BOOL isAppBackgrounded) {
if ([self.deviceCache isPurchaserInfoCacheStaleForAppUserID:self.appUserID isAppBackgrounded:isAppBackgrounded]) {
RCDebugLog(@"PurchaserInfo cache is stale, updating caches");
[self fetchAndCachePurchaserInfoWithCompletion:nil isAppBackgrounded:isAppBackgrounded];
}
if ([self.deviceCache isOfferingsCacheStaleWithIsAppBackgrounded:isAppBackgrounded]) {
RCDebugLog(@"Offerings cache is stale, updating caches");
[self updateOfferingsCache:nil isAppBackgrounded:isAppBackgrounded];
}
}];
Comment on lines +827 to +836
Copy link
Member Author

Choose a reason for hiding this comment

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

would love to move all this stuff into a new class, but it feels too risky for this particular feature

}

- (RCPurchaserInfo *)readPurchaserInfoFromCache {
Expand Down Expand Up @@ -863,25 +868,30 @@ - (void)cachePurchaserInfo:(RCPurchaserInfo *)info forAppUserID:(NSString *)appU
}

- (void)updateAllCachesWithCompletionBlock:(nullable RCReceivePurchaserInfoBlock)completion {
[self fetchAndCachePurchaserInfoWithCompletion:completion];
[self updateOfferingsCache:nil];
[self.systemInfo isApplicationBackgroundedWithCompletion:^(BOOL isAppBackgrounded) {
[self fetchAndCachePurchaserInfoWithCompletion:completion isAppBackgrounded:isAppBackgrounded];
[self updateOfferingsCache:nil isAppBackgrounded:isAppBackgrounded];
}];
}

- (void)fetchAndCachePurchaserInfoWithCompletion:(nullable RCReceivePurchaserInfoBlock)completion {
[self.deviceCache setPurchaserInfoCacheTimestampToNow];
- (void)fetchAndCachePurchaserInfoWithCompletion:(nullable RCReceivePurchaserInfoBlock)completion
isAppBackgrounded:(BOOL)isAppBackgrounded {
NSString *appUserID = self.identityManager.currentAppUserID;
[self.backend getSubscriberDataWithAppUserID:appUserID
completion:^(RCPurchaserInfo * _Nullable info,
NSError * _Nullable error) {
if (error == nil) {
[self cachePurchaserInfo:info forAppUserID:appUserID];
[self sendUpdatedPurchaserInfoToDelegateIfChanged:info];
} else {
[self.deviceCache clearPurchaserInfoCacheTimestamp];
}

CALL_IF_SET_ON_MAIN_THREAD(completion, info, error);
}];
[self.deviceCache setPurchaserInfoCacheTimestampToNowForAppUserID:appUserID];
[self.operationDispatcher dispatchOnWorkerThreadWithRandomDelay:isAppBackgrounded block:^{
[self.backend getSubscriberDataWithAppUserID:appUserID
completion:^(RCPurchaserInfo *_Nullable info,
aboedo marked this conversation as resolved.
Show resolved Hide resolved
NSError *_Nullable error) {
if (error == nil) {
[self cachePurchaserInfo:info forAppUserID:appUserID];
[self sendUpdatedPurchaserInfoToDelegateIfChanged:info];
} else {
[self.deviceCache clearPurchaserInfoCacheTimestampForAppUserID:appUserID];
}

CALL_IF_SET_ON_MAIN_THREAD(completion, info, error);
}];
}];
}

- (void)performOnEachProductIdentifierInOfferings:(NSDictionary *)offeringsData
Expand All @@ -894,31 +904,34 @@ - (void)performOnEachProductIdentifierInOfferings:(NSDictionary *)offeringsData
}

- (void)offeringsWithCompletionBlock:(RCReceiveOfferingsBlock)completion {
if (self.deviceCache.cachedOfferings) {
RCDebugLog(@"Vending offerings from cache");
CALL_IF_SET_ON_MAIN_THREAD(completion, self.deviceCache.cachedOfferings, nil);
if (self.deviceCache.isOfferingsCacheStale) {
RCDebugLog(@"Offerings cache is stale, updating cache");
[self updateOfferingsCache:nil];
[self.systemInfo isApplicationBackgroundedWithCompletion:^(BOOL isAppBackgrounded) {
if (self.deviceCache.cachedOfferings) {
RCDebugLog(@"Vending offerings from cache");
CALL_IF_SET_ON_MAIN_THREAD(completion, self.deviceCache.cachedOfferings, nil);
if ([self.deviceCache isOfferingsCacheStaleWithIsAppBackgrounded:isAppBackgrounded]) {
RCDebugLog(@"Offerings cache is stale, updating cache");
[self updateOfferingsCache:nil isAppBackgrounded:isAppBackgrounded];
}
} else {
RCDebugLog(@"No cached offerings, fetching");
[self updateOfferingsCache:completion isAppBackgrounded:isAppBackgrounded];
}
} else {
RCDebugLog(@"No cached offerings, fetching");
[self updateOfferingsCache:completion];
}
}];
}

- (void)updateOfferingsCache:(nullable RCReceiveOfferingsBlock)completion {
- (void)updateOfferingsCache:(nullable RCReceiveOfferingsBlock)completion isAppBackgrounded:(BOOL)isAppBackgrounded {
[self.deviceCache setOfferingsCacheTimestampToNow];
__weak typeof(self) weakSelf = self;
Copy link
Contributor

Choose a reason for hiding this comment

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

I remember you added this a while ago. We don't need it anymore?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think we needed it in the first place - I believe I made a mistake in thinking that there was a retention cycle there, but really, the block is used only here and the completion block isn't stored elsewhere, so as soon as the completion is executed, it dies and the block is released, then the instance is ready to be released.

The case handled here is not trivial, though:

if you call reset after calling offerings and the callback hasn't returned:

  • if you're doing this weak referencing (current case), the purchases instance is killed, it no longer holds on to the completion block, and completion isn't called.
  • if you're doing this with strong referencing (it's the default, and what we do for almost all other methods), the purchases instance lives until the completion block is called, which might lead to some unexpected behavior if there are any values in memory for the instance that are outdated after the reset.

I think we should at some point actually audit this behavior and check out what happens after a reset with some of these completion blocks.

As of right now, I'm not sure completion not returning is better or worse than it returning but potentially messing up if there's anything outdated in memory from the previous purchases instance.

[self.backend getOfferingsForAppUserID:self.appUserID
completion:^(NSDictionary *data, NSError *error) {
__strong typeof(self) strongSelf = weakSelf;
if (error != nil) {
[strongSelf handleOfferingsUpdateError:error completion:completion];
return;
}
[strongSelf handleOfferingsBackendResultWithData:data completion:completion];
}];
[self.operationDispatcher dispatchOnWorkerThreadWithRandomDelay:isAppBackgrounded block:^{
[self.backend getOfferingsForAppUserID:self.appUserID
completion:^(NSDictionary *data, NSError *error) {
if (error != nil) {
[self handleOfferingsUpdateError:error completion:completion];
return;
}
[self handleOfferingsBackendResultWithData:data completion:completion];
}];
}];

}

- (void)handleOfferingsBackendResultWithData:(NSDictionary *)data completion:(RCReceiveOfferingsBlock)completion {
Expand Down