Skip to content

Commit dd00263

Browse files
paulb777christibbs
authored andcommitted
Fix FIAM test flakes and exposed library bugs (#3101)
* Fix FIAM test flakes and resulting library bugs * style * Fix naming of initWithExpireAdfterInSeconds:withTimeFetcher:withCacheFile. Move mock setup of mockTimeFetcher prior to initializing FIRIAMClearcutLogStorage * FIRIAMClearcutUploader: Only schedule next log send on init if there are logs that haven't been sent at that time. Early return from scheduling if there's already a pending send * Fix whitespace * Make cachePath in `initWithExpireAfterInSeconds:withTimeFetcher:cachePath:` nullable
1 parent 9703cae commit dd00263

File tree

4 files changed

+97
-56
lines changed

4 files changed

+97
-56
lines changed

Firebase/InAppMessaging/Analytics/FIRIAMClearcutLogStorage.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@ NS_ASSUME_NONNULL_BEGIN
3434
// attempted.
3535

3636
@interface FIRIAMClearcutLogStorage : NSObject
37+
- (instancetype)initWithExpireAfterInSeconds:(NSInteger)expireInSeconds
38+
withTimeFetcher:(id<FIRIAMTimeFetcher>)timeFetcher
39+
cachePath:(nullable NSString *)cachePath;
40+
3741
- (instancetype)initWithExpireAfterInSeconds:(NSInteger)expireInSeconds
3842
withTimeFetcher:(id<FIRIAMTimeFetcher>)timeFetcher;
3943

Firebase/InAppMessaging/Analytics/FIRIAMClearcutLogStorage.m

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,8 @@ + (NSString *)determineCacheFilePath {
8282
}
8383

8484
- (instancetype)initWithExpireAfterInSeconds:(NSInteger)expireInSeconds
85-
withTimeFetcher:(id<FIRIAMTimeFetcher>)timeFetcher {
85+
withTimeFetcher:(id<FIRIAMTimeFetcher>)timeFetcher
86+
cachePath:(nullable NSString *)cachePath {
8687
if (self = [super init]) {
8788
_records = [[NSMutableArray alloc] init];
8889
_timeFetcher = timeFetcher;
@@ -92,7 +93,7 @@ - (instancetype)initWithExpireAfterInSeconds:(NSInteger)expireInSeconds
9293
name:UIApplicationWillResignActiveNotification
9394
object:nil];
9495
@try {
95-
[self loadFromCachePath:nil];
96+
[self loadFromCachePath:cachePath];
9697
} @catch (NSException *exception) {
9798
FIRLogWarning(kFIRLoggerInAppMessaging, @"I-IAM230004",
9899
@"Non-fatal exception in loading persisted clearcut log records: %@.",
@@ -102,6 +103,13 @@ - (instancetype)initWithExpireAfterInSeconds:(NSInteger)expireInSeconds
102103
return self;
103104
}
104105

106+
- (instancetype)initWithExpireAfterInSeconds:(NSInteger)expireInSeconds
107+
withTimeFetcher:(id<FIRIAMTimeFetcher>)timeFetcher {
108+
return [self initWithExpireAfterInSeconds:expireInSeconds
109+
withTimeFetcher:timeFetcher
110+
cachePath:nil];
111+
}
112+
105113
- (void)appWillBecomeInactive {
106114
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0ul), ^{
107115
[self saveIntoCacheWithPath:nil];

Firebase/InAppMessaging/Analytics/FIRIAMClearcutUploader.m

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,11 @@ - (instancetype)initWithRequestSender:(FIRIAMClearcutHttpRequestSender *)request
105105
_nextValidSendTimeInMills = (int64_t)
106106
[_userDefaults doubleForKey:FIRIAM_UserDefaultsKeyForNextValidClearcutUploadTimeInMills];
107107

108-
// seed the first send upon SDK start-up
109-
[self scheduleNextSend];
108+
NSArray<FIRIAMClearcutLogRecord *> *availableLogs =
109+
[logStorage popStillValidRecordsForUpTo:strategy.batchSendSize];
110+
if (availableLogs.count) {
111+
[self scheduleNextSend];
112+
}
110113

111114
FIRLogDebug(kFIRLoggerInAppMessaging, @"I-IAM260001",
112115
@"FIRIAMClearcutUploader created with strategy as %@", self.strategy);
@@ -133,20 +136,20 @@ - (void)addNewLogRecord:(FIRIAMClearcutLogRecord *)record {
133136
}
134137

135138
- (void)attemptUploading {
136-
NSArray<FIRIAMClearcutLogRecord *> *availbleLogs =
139+
NSArray<FIRIAMClearcutLogRecord *> *availableLogs =
137140
[self.logStorage popStillValidRecordsForUpTo:self.strategy.batchSendSize];
138141

139-
if (availbleLogs.count > 0) {
142+
if (availableLogs.count > 0) {
140143
FIRLogDebug(kFIRLoggerInAppMessaging, @"I-IAM260011", @"Deliver %d clearcut records",
141-
(int)availbleLogs.count);
144+
(int)availableLogs.count);
142145
[self.requestSender
143-
sendClearcutHttpRequestForLogs:availbleLogs
146+
sendClearcutHttpRequestForLogs:availableLogs
144147
withCompletion:^(BOOL success, BOOL shouldRetryLogs,
145148
int64_t waitTimeInMills) {
146149
if (success) {
147150
FIRLogDebug(kFIRLoggerInAppMessaging, @"I-IAM260003",
148151
@"Delivering %d clearcut records was successful",
149-
(int)availbleLogs.count);
152+
(int)availableLogs.count);
150153
// make sure the effective wait time is between two bounds
151154
// defined in strategy
152155
waitTimeInMills =
@@ -159,7 +162,7 @@ - (void)attemptUploading {
159162
FIRLogDebug(kFIRLoggerInAppMessaging, @"I-IAM260004",
160163
@"Failed to attempt the delivery of %d clearcut "
161164
@"records and should-retry for them is %@",
162-
(int)availbleLogs.count, shouldRetryLogs ? @"YES" : @"NO");
165+
(int)availableLogs.count, shouldRetryLogs ? @"YES" : @"NO");
163166
if (shouldRetryLogs) {
164167
/**
165168
* Note that there is a chance that the app crashes before we can
@@ -170,7 +173,7 @@ - (void)attemptUploading {
170173
*/
171174
FIRLogDebug(kFIRLoggerInAppMessaging, @"I-IAM260007",
172175
@"Push failed log records back to storage");
173-
[self.logStorage pushRecords:availbleLogs];
176+
[self.logStorage pushRecords:availableLogs];
174177
}
175178

176179
waitTimeInMills = (int64_t)self.strategy.failureBackoffTimeInMills;
@@ -207,10 +210,7 @@ - (void)attemptUploading {
207210
- (void)scheduleNextSend {
208211
@synchronized(self) {
209212
if (_nextSendScheduled) {
210-
// already scheduled next send, don't do it again
211213
return;
212-
} else {
213-
_nextSendScheduled = YES;
214214
}
215215
}
216216

@@ -228,6 +228,9 @@ - (void)scheduleNextSend {
228228
_queue, ^{
229229
[self attemptUploading];
230230
});
231+
@synchronized(self) {
232+
_nextSendScheduled = YES;
233+
}
231234
}
232235

233236
@end

0 commit comments

Comments
 (0)