Skip to content

Commit

Permalink
Fix bug where app launch messages weren't appearing on iOS (#3237)
Browse files Browse the repository at this point in the history
* Fix bug where messages with `app_launch` as a trigger weren't being parsed correctly

* Add a call to display executor check for app launch messages upon completion of initial app launch message fetch.

* Verify that the check for app launch messages gets called on initial app launch fetch

* Whitespace fix

* Fix duplicated error codes in checkAndDisplayNextAppLaunchMessage

* Refactor out logic for looking into app launch/foreground trigger messages to share more code

* Whitespace fix
  • Loading branch information
christibbs committed Jun 26, 2019
1 parent a1fc9b2 commit 16fbcf6
Show file tree
Hide file tree
Showing 14 changed files with 114 additions and 22 deletions.
3 changes: 3 additions & 0 deletions Firebase/InAppMessaging/Data/FIRIAMFetchResponseParser.m
Expand Up @@ -101,9 +101,12 @@ - (instancetype)initWithTimeFetcher:(id<FIRIAMTimeFetcher>)timeFetcher {
NSMutableArray<FIRIAMDisplayTriggerDefinition *> *triggers = [[NSMutableArray alloc] init];

for (NSDictionary *nextTriggerCondition in triggerConditions) {
// Handle app_launch and on_foreground cases.
if (nextTriggerCondition[@"fiamTrigger"]) {
if ([nextTriggerCondition[@"fiamTrigger"] isEqualToString:@"ON_FOREGROUND"]) {
[triggers addObject:[[FIRIAMDisplayTriggerDefinition alloc] initForAppForegroundTrigger]];
} else if ([nextTriggerCondition[@"fiamTrigger"] isEqualToString:@"APP_LAUNCH"]) {
[triggers addObject:[[FIRIAMDisplayTriggerDefinition alloc] initForAppLaunchTrigger]];
}
} else if ([nextTriggerCondition[@"event"] isKindOfClass:[NSDictionary class]]) {
NSDictionary *triggeringEvent = (NSDictionary *)nextTriggerCondition[@"event"];
Expand Down
6 changes: 4 additions & 2 deletions Firebase/InAppMessaging/Data/FIRIAMMessageDefinition.h
Expand Up @@ -15,6 +15,7 @@
*/
#import <Foundation/Foundation.h>

#import "FIRIAMDisplayTriggerDefinition.h"
#import "FIRIAMMessageRenderData.h"

@class FIRIAMDisplayTriggerDefinition;
Expand Down Expand Up @@ -52,8 +53,9 @@ NS_ASSUME_NONNULL_BEGIN
- (BOOL)messageHasExpired;
- (BOOL)messageHasStarted;

// should this message be rendered when the app gets foregrounded?
- (BOOL)messageRenderedOnAppForegroundEvent;
// should this message be rendered given the FIAM trigger type? only use this method for app launch
// and foreground trigger, use messageRenderedOnAnalyticsEvent: for analytics triggers
- (BOOL)messageRenderedOnTrigger:(FIRIAMRenderTrigger)trigger;
// should this message be rendered when a given analytics event is fired?
- (BOOL)messageRenderedOnAnalyticsEvent:(NSString *)eventName;
@end
Expand Down
5 changes: 2 additions & 3 deletions Firebase/InAppMessaging/Data/FIRIAMMessageDefinition.m
Expand Up @@ -15,7 +15,6 @@
*/

#import "FIRIAMMessageDefinition.h"
#import "FIRIAMDisplayTriggerDefinition.h"

@implementation FIRIAMMessageRenderData

Expand Down Expand Up @@ -60,9 +59,9 @@ - (BOOL)messageHasExpired {
return self.endTime < [[NSDate date] timeIntervalSince1970];
}

- (BOOL)messageRenderedOnAppForegroundEvent {
- (BOOL)messageRenderedOnTrigger:(FIRIAMRenderTrigger)trigger {
for (FIRIAMDisplayTriggerDefinition *nextTrigger in self.renderTriggers) {
if (nextTrigger.triggerType == FIRIAMRenderTriggerOnAppForeground) {
if (nextTrigger.triggerType == trigger) {
return YES;
}
}
Expand Down
Expand Up @@ -17,6 +17,7 @@
#import <Foundation/Foundation.h>

typedef NS_ENUM(NSInteger, FIRIAMRenderTrigger) {
FIRIAMRenderTriggerOnAppLaunch,
FIRIAMRenderTriggerOnAppForeground,
FIRIAMRenderTriggerOnFirebaseAnalyticsEvent
};
Expand All @@ -28,6 +29,7 @@ NS_ASSUME_NONNULL_BEGIN
// applicable only when triggerType == FIRIAMRenderTriggerOnFirebaseAnalyticsEvent
@property(nonatomic, copy, nullable, readonly) NSString *firebaseEventName;

- (instancetype)initForAppLaunchTrigger;
- (instancetype)initForAppForegroundTrigger;
- (instancetype)initWithFirebaseAnalyticEvent:(NSString *)title;
@end
Expand Down
Expand Up @@ -17,6 +17,14 @@
#import "FIRIAMDisplayTriggerDefinition.h"

@implementation FIRIAMDisplayTriggerDefinition

- (instancetype)initForAppLaunchTrigger {
if (self = [super init]) {
_triggerType = FIRIAMRenderTriggerOnAppLaunch;
}
return self;
}

- (instancetype)initForAppForegroundTrigger {
if (self = [super init]) {
_triggerType = FIRIAMRenderTriggerOnAppForeground;
Expand Down
2 changes: 2 additions & 0 deletions Firebase/InAppMessaging/Flows/FIRIAMDisplayExecutor.h
Expand Up @@ -49,6 +49,8 @@ NS_ASSUME_NONNULL_BEGIN
activityLogger:(FIRIAMActivityLogger *)activityLogger
analyticsEventLogger:(id<FIRIAMAnalyticsEventLogger>)analyticsEventLogger;

// Check and display next in-app message eligible for app launch trigger
- (void)checkAndDisplayNextAppLaunchMessage;
// Check and display next in-app message eligible for app open trigger
- (void)checkAndDisplayNextAppForegroundMessage;
// Check and display next in-app message eligible for analytics event trigger with given event name.
Expand Down
43 changes: 43 additions & 0 deletions Firebase/InAppMessaging/Flows/FIRIAMDisplayExecutor.m
Expand Up @@ -604,6 +604,49 @@ - (BOOL)enoughIntervalFromLastDisplay {
return intervalFromLastDisplayInSeconds >= self.setting.displayMinIntervalInMinutes * 60.0;
}

- (void)checkAndDisplayNextAppLaunchMessage {
// synchronizing on self so that we won't potentially enter the render flow from two
// threads.
@synchronized(self) {
if (!self.messageDisplayComponent) {
FIRLogDebug(kFIRLoggerInAppMessaging, @"I-IAM400028",
@"Message display component is not present yet. No display should happen.");
return;
}

if (self.suppressMessageDisplay) {
FIRLogDebug(kFIRLoggerInAppMessaging, @"I-IAM400029",
@"Message display is being suppressed. No regular message rendering.");
return;
}

if (self.isMsgBeingDisplayed) {
FIRLogDebug(kFIRLoggerInAppMessaging, @"I-IAM400030",
@"An in-app message display is in progress, do not over-display on top of it.");
return;
}

if ([self.messageCache hasTestMessage] || [self enoughIntervalFromLastDisplay]) {
// We can display test messages anytime or display regular messages when
// the display time interval has been reached
FIRIAMMessageDefinition *nextAppLaunchMessage = [self.messageCache nextOnAppLaunchDisplayMsg];

if (nextAppLaunchMessage) {
[self displayForMessage:nextAppLaunchMessage
triggerType:FIRInAppMessagingDisplayTriggerTypeOnAnalyticsEvent];
self.lastDisplayTime = [self.timeFetcher currentTimestampInSeconds];
} else {
FIRLogDebug(kFIRLoggerInAppMessaging, @"I-IAM400040",
@"No appropriate in-app message detected for display.");
}
} else {
FIRLogDebug(kFIRLoggerInAppMessaging, @"I-IAM400041",
@"Minimal display interval of %lf seconds has not been reached yet.",
self.setting.displayMinIntervalInMinutes * 60.0);
}
}
}

- (void)checkAndDisplayNextAppForegroundMessage {
// synchronizing on self so that we won't potentially enter the render flow from two
// threads: example like showing analytics triggered message and a regular app open
Expand Down
3 changes: 2 additions & 1 deletion Firebase/InAppMessaging/Flows/FIRIAMFetchFlow.h
Expand Up @@ -54,6 +54,7 @@ typedef void (^FIRIAMFetchMessageCompletionHandler)(

// Triggers a potential fetch of in-app messaging from the source. It would check and respect the
// the fetchMinIntervalInMinutes defined in setting
- (void)checkAndFetch;
- (void)checkAndFetchForInitialAppLaunch:(BOOL)forInitialAppLaunch;

@end
NS_ASSUME_NONNULL_END
12 changes: 11 additions & 1 deletion Firebase/InAppMessaging/Flows/FIRIAMFetchFlow.m
Expand Up @@ -19,6 +19,7 @@
#import "FIRCore+InAppMessaging.h"
#import "FIRIAMClearcutLogger.h"
#import "FIRIAMFetchFlow.h"
#import "FIRIAMRuntimeManager.h"

@implementation FIRIAMFetchSetting
@end
Expand Down Expand Up @@ -124,7 +125,7 @@ - (void)handleSuccessullyFetchedMessages:(NSArray<FIRIAMMessageDefinition *> *)m
nextFetchWaitTime:fetchWaitTime];
}

- (void)checkAndFetch {
- (void)checkAndFetchForInitialAppLaunch:(BOOL)forInitialAppLaunch {
NSTimeInterval intervalFromLastFetchInSeconds =
[self.timeFetcher currentTimestampInSeconds] - self.fetchBookKeeper.lastFetchTime;

Expand Down Expand Up @@ -229,6 +230,10 @@ - (void)checkAndFetch {
[self handleSuccessullyFetchedMessages:messages
withFetchWaitTime:nextFetchWaitTime
requestImpressions:impressions];

if (forInitialAppLaunch) {
[self checkForAppLaunchMessage];
}
}
// Send this regardless whether fetch is successful or not.
[self sendFetchIsDoneNotification];
Expand All @@ -250,4 +255,9 @@ - (void)checkAndFetch {
[self.activityLogger addLogRecord:record];
}
}

- (void)checkForAppLaunchMessage {
[[FIRIAMRuntimeManager getSDKRuntimeInstance]
.displayExecutor checkAndDisplayNextAppLaunchMessage];
}
@end
Expand Up @@ -35,7 +35,7 @@ - (void)appWillEnterForeground:(UIApplication *)application {
// to a concurrent global queue instead of serial queue since app open event won't happen at
// fast speed to cause race conditions
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0ul), ^{
[self checkAndFetch];
[self checkAndFetchForInitialAppLaunch:NO];
});
}

Expand Down
2 changes: 2 additions & 0 deletions Firebase/InAppMessaging/Flows/FIRIAMMessageClientCache.h
Expand Up @@ -74,6 +74,8 @@ NS_ASSUME_NONNULL_BEGIN
// nextOnFirebaseAnalyticEventDisplayMsg to fetch the next eligible message and use
// removeMessageWithId to remove it from cache once the message has been correctly rendered

// Fetch next eligible messages that are appropriate for display at app launch time
- (nullable FIRIAMMessageDefinition *)nextOnAppLaunchDisplayMsg;
// Fetch next eligible messages that are appropriate for display at app open time
- (nullable FIRIAMMessageDefinition *)nextOnAppOpenDisplayMsg;
// Fetch next eligible message that matches the event triggering condition
Expand Down
25 changes: 18 additions & 7 deletions Firebase/InAppMessaging/Flows/FIRIAMMessageClientCache.m
Expand Up @@ -126,13 +126,11 @@ - (BOOL)hasTestMessage {
return self.testMessages.count > 0;
}

- (nullable FIRIAMMessageDefinition *)nextOnAppOpenDisplayMsg {
// search from the start to end in the list (which implies the display priority) for the
// first match (some messages in the cache may not be eligible for the current display
// message fetch
NSSet<NSString *> *impressionSet =
[NSSet setWithArray:[self.bookKeeper getMessageIDsFromImpressions]];
- (nullable FIRIAMMessageDefinition *)nextOnAppLaunchDisplayMsg {
return [self nextMessageForTrigger:FIRIAMRenderTriggerOnAppLaunch];
}

- (nullable FIRIAMMessageDefinition *)nextOnAppOpenDisplayMsg {
@synchronized(self) {
// always first check test message which always have higher prirority
if (self.testMessages.count > 0) {
Expand All @@ -143,12 +141,25 @@ - (nullable FIRIAMMessageDefinition *)nextOnAppOpenDisplayMsg {
@"Returning a test message for app foreground display");
return testMessage;
}
}

// otherwise check for a message from a published campaign
return [self nextMessageForTrigger:FIRIAMRenderTriggerOnAppForeground];
}

- (nullable FIRIAMMessageDefinition *)nextMessageForTrigger:(FIRIAMRenderTrigger)trigger {
// search from the start to end in the list (which implies the display priority) for the
// first match (some messages in the cache may not be eligible for the current display
// message fetch
NSSet<NSString *> *impressionSet =
[NSSet setWithArray:[self.bookKeeper getMessageIDsFromImpressions]];

@synchronized(self) {
for (FIRIAMMessageDefinition *next in self.regularMessages) {
// message being active and message not impressed yet
if ([next messageHasStarted] && ![next messageHasExpired] &&
![impressionSet containsObject:next.renderData.messageID] &&
[next messageRenderedOnAppForegroundEvent]) {
[next messageRenderedOnTrigger:trigger]) {
return next;
}
}
Expand Down
3 changes: 2 additions & 1 deletion Firebase/InAppMessaging/Runtime/FIRIAMRuntimeManager.m
Expand Up @@ -417,7 +417,8 @@ - (void)internalStartRuntimeWithSDKSettings:(FIRIAMSDKSettings *)settings {

// One-time triggering of checks for both fetch flow
// upon SDK/app startup.
[self.fetchOnAppForegroundFlow checkAndFetch];
[self.fetchOnAppForegroundFlow
checkAndFetchForInitialAppLaunch:YES];
} else {
FIRLogDebug(kFIRLoggerInAppMessaging, @"I-IAM180009",
@"No FIAM SDK startup due to settings.");
Expand Down
20 changes: 14 additions & 6 deletions InAppMessaging/Example/Tests/FIRIAMFetchFlowTests.m
Expand Up @@ -24,6 +24,11 @@
#import "FIRIAMMessageContentDataWithImageURL.h"
#import "FIRIAMSDKModeManager.h"

@interface FIRIAMFetchFlow (Testing)
// Expose to verify that this gets called on initial app launch fetch.
- (void)checkForAppLaunchMessage;
@end

@interface FIRIAMFetchFlowTests : XCTestCase
@property(nonatomic) FIRIAMFetchSetting *fetchSetting;
@property FIRIAMMessageClientCache *clientMessageCache;
Expand Down Expand Up @@ -202,7 +207,7 @@ - (void)testHappyPath {
fetchWaitTimeFromResponse,
[NSNull null], [NSNull null],
nil])]);
[self.flow checkAndFetch];
[self.flow checkAndFetchForInitialAppLaunch:NO];

// We expect m1 and m2 to be dumped into clientMessageCache.
NSArray<FIRIAMMessageDefinition *> *foundMessages = [self.clientMessageCache allRegularMessages];
Expand Down Expand Up @@ -238,7 +243,7 @@ - (void)testNoFetchDueToIntervalConstraint {
// We don't expect fetchMessages: for self.mockMessageFetcher to be triggred
OCMReject([self.mockMessageFetcher fetchMessagesWithImpressionList:[OCMArg any]
withCompletion:[OCMArg any]]);
[self.flow checkAndFetch];
[self.flow checkAndFetchForInitialAppLaunch:NO];

NSArray<FIRIAMMessageDefinition *> *foundMessages = [self.clientMessageCache allRegularMessages];
XCTAssertEqual(0, foundMessages.count);
Expand All @@ -259,7 +264,7 @@ - (void)testAlwaysFetchForNewlyInstalledMode {
OCMStub([self.mockBookkeeper nextFetchWaitTime]).andReturn(1000);
OCMStub([self.mockTimeFetcher currentTimestampInSeconds]).andReturn(100);

[self.flow checkAndFetch];
[self.flow checkAndFetchForInitialAppLaunch:YES];

// we expect m1 and m2 to be dumped into clientMessageCache
NSArray<FIRIAMMessageDefinition *> *foundMessages = [self.clientMessageCache allRegularMessages];
Expand All @@ -269,6 +274,9 @@ - (void)testAlwaysFetchForNewlyInstalledMode {

// we expect to register a fetch with sdk manager
OCMVerify([self.mockSDKModeManager registerOneMoreFetch]);

// we expect that the message cache is checked for app launch messages
OCMVerify([self.flow checkForAppLaunchMessage]);
}

// Fetch always in testing app instance mode
Expand All @@ -285,7 +293,7 @@ - (void)testAlwaysFetchForTestingAppInstanceMode {
OCMStub([self.mockBookkeeper nextFetchWaitTime]).andReturn(1000);
OCMStub([self.mockTimeFetcher currentTimestampInSeconds]).andReturn(100);

[self.flow checkAndFetch];
[self.flow checkAndFetchForInitialAppLaunch:NO];

// we expect m1 and m2 to be dumped into clientMessageCache
NSArray<FIRIAMMessageDefinition *> *foundMessages = [self.clientMessageCache allRegularMessages];
Expand Down Expand Up @@ -313,7 +321,7 @@ - (void)testTurnIntoTestigModeOnSeeingTestMessage {
// 100 seconds is larger than FETCH_MIN_INTERVALS minutes
OCMStub([self.mockTimeFetcher currentTimestampInSeconds]).andReturn(100);

[self.flow checkAndFetch];
[self.flow checkAndFetchForInitialAppLaunch:NO];

// Expecting turning sdk mode into a testing instance
OCMVerify([self.mockSDKModeManager becomeTestingInstance]);
Expand All @@ -335,6 +343,6 @@ - (void)testNotTurningIntoTestingModeIfAlreadyInTestingMode {
OCMStub([self.mockTimeFetcher currentTimestampInSeconds]).andReturn(1000);
OCMReject([self.mockSDKModeManager becomeTestingInstance]);

[self.flow checkAndFetch];
[self.flow checkAndFetchForInitialAppLaunch:NO];
}
@end

0 comments on commit 16fbcf6

Please sign in to comment.