Skip to content

Commit 662e3da

Browse files
authored
Added some extra handling for outcomes (#864)
* Added some extra handling for outcomes * Prevent clicked notifications in the foreground from being apart of the current session * Added override functionality before session 30 second min, only lower sessions can be overridden by upper sessions * Min time for focus and session is only in OneSignal.java * Caching current session now so when the user quits the app and comes back we might want to stay in the current session * Made the 5 second direct session timer back to 30 seconds * Unit test was failing after adjusting outcomes work * Needed to pause activity to background the app * No DIRECT session could come from foreground click * Updating unit tests for outcomes * Changed attribution window code to use System.currentTimeMillis() * Added test scenarios for new sessions, override sessions, and attribution window * Changed some formatting in current outcome unit tests * Added AppEntryState tracking and changing outcome flags for unit tests * AppEntryAction is an enum with 3 states: APP_OPEN, APP_CLOSE, NOTIFICATION_OPEN * appEntryState keeps track form OneSignal.java how we entered an application and will decide what type of session we currently are involved in * Modified OutcomeRemoteParams in unit tests so we can choose our own flags for testing purposes * Added default for appEntryState and cleaned up session checking * Fixed comment for session restart * Fixed checks in OSSessionManager * We want to check session and data in separate places sometimes * Now we have hasDirectNotification and hasIndirectNotifications along side isDirectSession and isIndirectSession * Added helpers for checking appEntryState and session * Methods for returning correct boolean associated with state check for AppEntryAction enum and Session enum * Cleaned up tests and code so they use enum helpers * Added OutcomeEventIntegrationTests * Made a package private helper for getting OSSessionManager from OneSignal used to help test integration of outcomes in app sessions and focus * Cleaned up outcomes integration tests in MainOneSignalClassRunner * Move on_focus logic into it's own class * Moved parts of on_focus logic from OneSignal and OneSignalSyncServiceUtils to a new FocusTimeController class - Good amount as also refactored in the move! - No functionally changes yet * FocusTimeController will be split up in future commits, there is a large TODO block at the top of what is left. * The shouldSyncPendingChangesFromSyncService test fails but is the only one. - Since a number of other changes are happening to FocusTimeController in this branch I held off on fixing this one. * Tracking attributed time on session ending * Refactored OSSessionManager.setSession to require notificationId(s). - This way it can fire off an event of the before state so the on_focus can subscribe and know the session just ended to make the call. - This means notificationId and notificationIds will only be set via the setSession * WIP notes; * * Still need to add a number of tests. * Fixed cold start on_focus sync * Moved the initialization of OSSessionManager to set setAppContext as it need this to load any caches values * Misc clean up based on PR review. * Added case where indirect goes to indirect on new session * Added unit testing for this scenario * Also cleaned up OutcomeEventIntegrationTests so its easier to click and receive notifications * Minor clean up with comments and naming * Update OutcomesUtils.java * Forgot to refactor name for method where its being used * Removed any public where not needed in OneSignal.java * Also added null check for sessionManager when clicking notif * on_focus included background time on click * Fixed case where background time could be included when tapping on a notification in the background. - To prevent this we clear the time stamp when the app goes into the background. * Added package private helper for getting OutcomeSettings * Fixed unit tests using OutcomeSettings * OutcomeSettings needed to be package private * Added test to validate on_focus is sent when overriding session * Example is DIRECT session overriding an INDIRECT session * Being in an INDIRECT session, leaving the app and clicking a notification should trigger a DIRECT session and the on_focus job for the INDIRECT session should fire * Fixed email creating double sessions for outcomes * Fixed session upgrade from indirect to direct * Fixed testDirectSession_willOverrideIndirectSession_whenAppIsInBackground * Some clean up to FocusTimeController + attemptSessionUpgrade comments * Fixed OSSessionManager to be package-private * Cleaned up completed TODOs * Misc clean up * Added getters for initDone and appEntryState
1 parent 2c31f4a commit 662e3da

24 files changed

+1618
-580
lines changed
Lines changed: 267 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,267 @@
1+
package com.onesignal;
2+
3+
import android.os.SystemClock;
4+
import android.support.annotation.NonNull;
5+
import android.support.annotation.Nullable;
6+
import android.support.annotation.WorkerThread;
7+
8+
import org.json.JSONException;
9+
import org.json.JSONObject;
10+
11+
import java.util.Arrays;
12+
import java.util.List;
13+
import java.util.concurrent.atomic.AtomicBoolean;
14+
15+
16+
/**
17+
* Entry points that could cause on_focus to fire:
18+
* 1. OSSessionManager.session changed (onSessionEnded) - Send any attributed session time
19+
* 2. App is foregrounded (appForegrounded) - Set start focused time
20+
* 3. App is backgrounded (appBackgrounded) - Kick off job to sync when session ends
21+
*/
22+
23+
24+
class FocusTimeController {
25+
// Only present if app is currently in focus.
26+
@Nullable private Long timeFocusedAtMs;
27+
28+
private static FocusTimeController sInstance;
29+
30+
private List<FocusTimeProcessorBase> focusTimeProcessors =
31+
Arrays.asList(new FocusTimeProcessorUnattributed(), new FocusTimeProcessorAttributed());
32+
33+
private enum FocusEventType {
34+
BACKGROUND,
35+
END_SESSION
36+
}
37+
38+
private FocusTimeController() { }
39+
public static synchronized FocusTimeController getInstance() {
40+
if (sInstance == null)
41+
sInstance = new FocusTimeController();
42+
return sInstance;
43+
}
44+
45+
void appForegrounded() {
46+
timeFocusedAtMs = SystemClock.elapsedRealtime();
47+
}
48+
49+
void appBackgrounded() {
50+
giveProcessorsValidFocusTime(OneSignal.getSessionManager().getSessionResult(), FocusEventType.BACKGROUND);
51+
timeFocusedAtMs = null;
52+
}
53+
54+
void onSessionEnded(@NonNull OSSessionManager.SessionResult lastSessionResult) {
55+
final FocusEventType focusEventType = FocusEventType.END_SESSION;
56+
boolean hadValidTime = giveProcessorsValidFocusTime(lastSessionResult, focusEventType);
57+
58+
// If there is no in focus time to be added we just need to send the time from the last session that just ended.
59+
if (!hadValidTime) {
60+
for (FocusTimeProcessorBase focusTimeProcessor : focusTimeProcessors)
61+
focusTimeProcessor.sendUnsentTimeNow(focusEventType);
62+
}
63+
}
64+
65+
void doBlockingBackgroundSyncOfUnsentTime() {
66+
if (OneSignal.isForeground())
67+
return;
68+
69+
for(FocusTimeProcessorBase focusTimeProcessor : focusTimeProcessors)
70+
focusTimeProcessor.syncUnsentTimeFromSyncJob();
71+
}
72+
73+
private boolean giveProcessorsValidFocusTime(@NonNull OSSessionManager.SessionResult lastSessionResult, @NonNull FocusEventType focusType) {
74+
Long timeElapsed = getTimeFocusedElapsed();
75+
if (timeElapsed == null)
76+
return false;
77+
78+
for(FocusTimeProcessorBase focusTimeProcessor : focusTimeProcessors)
79+
focusTimeProcessor.addTime(timeElapsed, lastSessionResult.session, focusType);
80+
return true;
81+
}
82+
83+
// Get time past since app was put into focus.
84+
// Will be null if time is invalid or 0
85+
private @Nullable Long getTimeFocusedElapsed() {
86+
// timeFocusedAtMs is cleared when the app goes into the background so we don't have a focus time
87+
if (timeFocusedAtMs == null)
88+
return null;
89+
90+
long timeElapsed = (long)(((SystemClock.elapsedRealtime() - timeFocusedAtMs) / 1_000d) + 0.5d);
91+
92+
// Time is invalid if below 1 or over a day
93+
if (timeElapsed < 1 || timeElapsed > 86_400)
94+
return null;
95+
return timeElapsed;
96+
}
97+
98+
private static class FocusTimeProcessorUnattributed extends FocusTimeProcessorBase {
99+
FocusTimeProcessorUnattributed() {
100+
MIN_ON_FOCUS_TIME_SEC = 60;
101+
PREF_KEY_FOR_UNSENT_TIME = OneSignalPrefs.PREFS_GT_UNSENT_ACTIVE_TIME;
102+
}
103+
104+
protected boolean timeTypeApplies(@NonNull OSSessionManager.Session session) {
105+
return session.isUnattributed() || session.isDisabled();
106+
}
107+
108+
protected void sendTime(@NonNull FocusEventType focusType) {
109+
// We only need to send unattributed focus time when the app goes out of focus.
110+
if (focusType.equals(FocusEventType.END_SESSION))
111+
return;
112+
113+
syncUnsentTimeOnBackgroundEvent();
114+
}
115+
}
116+
117+
private static class FocusTimeProcessorAttributed extends FocusTimeProcessorBase {
118+
FocusTimeProcessorAttributed() {
119+
MIN_ON_FOCUS_TIME_SEC = 1;
120+
PREF_KEY_FOR_UNSENT_TIME = OneSignalPrefs.PREFS_OS_UNSENT_ATTRIBUTED_ACTIVE_TIME;
121+
}
122+
123+
protected boolean timeTypeApplies(@NonNull OSSessionManager.Session session) {
124+
return session.isAttributed();
125+
}
126+
127+
protected void additionalFieldsToAddToOnFocusPayload(@NonNull JSONObject jsonBody) {
128+
OneSignal.getSessionManager().addSessionNotificationsIds(jsonBody);
129+
}
130+
131+
protected void sendTime(@NonNull FocusEventType focusType) {
132+
if (focusType.equals(FocusEventType.END_SESSION))
133+
syncOnFocusTime();
134+
else
135+
OneSignalSyncServiceUtils.scheduleSyncTask(OneSignal.appContext);
136+
}
137+
}
138+
139+
private static abstract class FocusTimeProcessorBase {
140+
// These values are set by child classes that inherit this base class
141+
protected long MIN_ON_FOCUS_TIME_SEC;
142+
protected @NonNull String PREF_KEY_FOR_UNSENT_TIME;
143+
144+
protected abstract boolean timeTypeApplies(@NonNull OSSessionManager.Session session);
145+
protected abstract void sendTime(@NonNull FocusEventType focusType);
146+
147+
@Nullable private Long unsentActiveTime = null;
148+
149+
private long getUnsentActiveTime() {
150+
if (unsentActiveTime == null) {
151+
unsentActiveTime = OneSignalPrefs.getLong(
152+
OneSignalPrefs.PREFS_ONESIGNAL,
153+
PREF_KEY_FOR_UNSENT_TIME,
154+
0
155+
);
156+
}
157+
OneSignal.Log(OneSignal.LOG_LEVEL.DEBUG, this.getClass().getSimpleName() + ":getUnsentActiveTime: " + unsentActiveTime);
158+
return unsentActiveTime;
159+
}
160+
161+
private void saveUnsentActiveTime(long time) {
162+
unsentActiveTime = time;
163+
OneSignal.Log(OneSignal.LOG_LEVEL.DEBUG, this.getClass().getSimpleName() + ":saveUnsentActiveTime: " + unsentActiveTime);
164+
OneSignalPrefs.saveLong(
165+
OneSignalPrefs.PREFS_ONESIGNAL,
166+
PREF_KEY_FOR_UNSENT_TIME,
167+
time
168+
);
169+
}
170+
171+
private void addTime(long time, @NonNull OSSessionManager.Session session, @NonNull FocusEventType focusType) {
172+
if (!timeTypeApplies(session))
173+
return;
174+
175+
long totalTime = getUnsentActiveTime() + time;
176+
saveUnsentActiveTime(totalTime);
177+
sendUnsentTimeNow(focusType);
178+
}
179+
180+
private void sendUnsentTimeNow(FocusEventType focusType) {
181+
if (!OneSignal.hasUserId())
182+
return;
183+
184+
sendTime(focusType);
185+
}
186+
187+
private boolean hasMinSyncTime() {
188+
return getUnsentActiveTime() >= MIN_ON_FOCUS_TIME_SEC;
189+
}
190+
191+
protected void syncUnsentTimeOnBackgroundEvent() {
192+
if (!hasMinSyncTime())
193+
return;
194+
// Schedule this sync in case app is killed before completing
195+
OneSignalSyncServiceUtils.scheduleSyncTask(OneSignal.appContext);
196+
syncOnFocusTime();
197+
}
198+
199+
private void syncUnsentTimeFromSyncJob() {
200+
if (hasMinSyncTime())
201+
syncOnFocusTime();
202+
}
203+
204+
@NonNull private final AtomicBoolean runningOnFocusTime = new AtomicBoolean();
205+
@WorkerThread
206+
protected void syncOnFocusTime() {
207+
if (runningOnFocusTime.get())
208+
return;
209+
210+
synchronized (runningOnFocusTime) {
211+
runningOnFocusTime.set(true);
212+
if (hasMinSyncTime())
213+
sendOnFocus(getUnsentActiveTime());
214+
runningOnFocusTime.set(false);
215+
}
216+
}
217+
218+
private void sendOnFocusToPlayer(@NonNull String userId, @NonNull JSONObject jsonBody) {
219+
OneSignalRestClient.ResponseHandler responseHandler = new OneSignalRestClient.ResponseHandler() {
220+
@Override
221+
void onFailure(int statusCode, String response, Throwable throwable) {
222+
OneSignal.logHttpError("sending on_focus Failed", statusCode, throwable, response);
223+
}
224+
225+
@Override
226+
void onSuccess(String response) {
227+
// TODO: PRE-EXISTING: This time is shared between the email + push player and
228+
// is cleared no matter which one is successful.
229+
// TODO: PRE-EXISTING: This could be clearing time more then was persisted while the network call was in flight
230+
saveUnsentActiveTime(0);
231+
}
232+
};
233+
String url = "players/" + userId + "/on_focus";
234+
OneSignalRestClient.postSync(url, jsonBody, responseHandler);
235+
}
236+
237+
// Override Optional
238+
protected void additionalFieldsToAddToOnFocusPayload(@NonNull JSONObject jsonBody) { }
239+
240+
private @NonNull JSONObject generateOnFocusPayload(long totalTimeActive) throws JSONException {
241+
JSONObject jsonBody = new JSONObject()
242+
.put("app_id", OneSignal.appId)
243+
.put("type", 1) // Always 1, where this type means do NOT increase session_count
244+
.put("state", "ping") // Always ping, other types are not used
245+
.put("active_time", totalTimeActive)
246+
.put("device_type", new OSUtils().getDeviceType());
247+
OneSignal.addNetType(jsonBody);
248+
return jsonBody;
249+
}
250+
251+
private void sendOnFocus(long totalTimeActive) {
252+
try {
253+
JSONObject jsonBody = generateOnFocusPayload(totalTimeActive);
254+
additionalFieldsToAddToOnFocusPayload(jsonBody);
255+
sendOnFocusToPlayer(OneSignal.getUserId(), jsonBody);
256+
257+
// For email we omit additionalFieldsToAddToOnFocusPayload as we don't want to add
258+
// outcome fields which would double report the session time
259+
if (OneSignal.hasEmailId())
260+
sendOnFocusToPlayer(OneSignal.getEmailId(), generateOnFocusPayload(totalTimeActive));
261+
}
262+
catch (JSONException t) {
263+
OneSignal.Log(OneSignal.LOG_LEVEL.ERROR, "Generating on_focus:JSON Failed.", t);
264+
}
265+
}
266+
}
267+
}

OneSignalSDK/onesignal/src/main/java/com/onesignal/GenerateNotification.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ private static OneSignalNotificationBuilder getBaseOneSignalNotificationBuilder(
263263

264264
if (notifJob.shownTimeStamp != null) {
265265
try {
266-
notifBuilder.setWhen(notifJob.shownTimeStamp * 1000L);
266+
notifBuilder.setWhen(notifJob.shownTimeStamp * 1_000L);
267267
} catch (Throwable t) {} // Can throw if an old android support lib is used.
268268
}
269269

OneSignalSDK/onesignal/src/main/java/com/onesignal/JSONUtils.java

Lines changed: 71 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,12 @@
1616

1717

1818
class JSONUtils {
19-
// Returns a JSONObject of the differences between cur and changedTo.
20-
// If baseOutput is added changes will be applied to this JSONObject.
21-
// includeFields will always be added to the returned JSONObject if they are in cur.
19+
20+
/**
21+
* Returns a JSONObject of the differences between cur and changedTo.
22+
* If baseOutput is added changes will be applied to this JSONObject.
23+
* includeFields will always be added to the returned JSONObject if they are in cur.
24+
*/
2225
static JSONObject generateJsonDiff(JSONObject cur, JSONObject changedTo, JSONObject baseOutput, Set<String> includeFields) {
2326
if (cur == null)
2427
return null;
@@ -158,7 +161,9 @@ static JSONObject getJSONObjectWithoutBlankValues(JSONObject jsonObject, String
158161
return jsonObjectToMapNonNull(json);
159162
}
160163

161-
// Converts a JSONObject into a Map, same as above however does NOT accept null values
164+
/**
165+
* Converts a JSONObject into a Map, same as above however does NOT accept null values
166+
*/
162167
private static @NonNull Map<String, Object> jsonObjectToMapNonNull(@NonNull JSONObject object) throws JSONException {
163168
Map<String, Object> map = new HashMap<>();
164169
Iterator<String> keysItr = object.keys();
@@ -170,7 +175,9 @@ static JSONObject getJSONObjectWithoutBlankValues(JSONObject jsonObject, String
170175
return map;
171176
}
172177

173-
// Converts a JSONArray into a List, returns null if a null value is passed in.
178+
/**
179+
* Converts a JSONArray into a List, returns null if a null value is passed in.
180+
*/
174181
static @Nullable List<Object> jsonArrayToList(@Nullable JSONArray array) throws JSONException {
175182
if (array == null)
176183
return null;
@@ -187,8 +194,10 @@ static JSONObject getJSONObjectWithoutBlankValues(JSONObject jsonObject, String
187194
return list;
188195
}
189196

190-
// Digs into any nested JSONObject or JSONArray to convert them to a List or Map
191-
// If object is another type is is returned back.
197+
/**
198+
* Digs into any nested JSONObject or JSONArray to convert them to a List or Map
199+
* If object is another type is is returned back.
200+
*/
192201
private static @NonNull Object convertNestedJSONType(@NonNull Object value) throws JSONException {
193202
if (value instanceof JSONObject)
194203
return jsonObjectToMapNonNull((JSONObject)value);
@@ -197,4 +206,59 @@ static JSONObject getJSONObjectWithoutBlankValues(JSONObject jsonObject, String
197206
return value;
198207
}
199208

209+
/**
210+
* Compare two JSONArrays too determine if they are equal or not
211+
*/
212+
static boolean compareJSONArrays(JSONArray jsonArray1, JSONArray jsonArray2) {
213+
// If both JSONArrays are null, they are equal
214+
if (jsonArray1 == null && jsonArray2 == null)
215+
return true;
216+
217+
// If one JSONArray is null but not the other, they are not equal
218+
if (jsonArray1 == null || jsonArray2 == null)
219+
return false;
220+
221+
// If one JSONArray is a different size then the other, they are not equal
222+
if (jsonArray1.length() != jsonArray2.length())
223+
return false;
224+
225+
try {
226+
L1 : for (int i = 0; i < jsonArray1.length(); i++) {
227+
for (int j = 0; j < jsonArray2.length(); j++) {
228+
Object obj1 = normalizeType(jsonArray1.get(i));
229+
Object obj2 = normalizeType(jsonArray2.get(j));
230+
// Make sure jsonArray1 current item exists somewhere inside jsonArray2
231+
// If item found continue looping
232+
if (obj1.equals(obj2))
233+
continue L1;
234+
}
235+
236+
// Could not find current item from jsonArray1 inside jsonArray2, so they are not equal
237+
return false;
238+
}
239+
} catch (JSONException e) {
240+
e.printStackTrace();
241+
242+
// Exception thrown, return false
243+
return false;
244+
}
245+
246+
// JSONArrays are equal
247+
return true;
248+
}
249+
250+
// Converts Java types that are equivalent in the JSON format to the same types.
251+
// This allows for assertEquals on two values from JSONObject.get to test values as long as it
252+
// returns in the same JSON output.
253+
public static Object normalizeType(Object object) {
254+
Class clazz = object.getClass();
255+
256+
if (clazz.equals(Integer.class))
257+
return Long.valueOf((Integer)object);
258+
if (clazz.equals(Float.class))
259+
return Double.valueOf((Float)object);
260+
261+
return object;
262+
}
263+
200264
}

OneSignalSDK/onesignal/src/main/java/com/onesignal/NotificationBundleProcessor.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ static void markRestoredNotificationAsDismissed(NotificationGenerationJob notifi
274274
// Clean up old records after 1 week.
275275
static void deleteOldNotifications(SQLiteDatabase writableDb) {
276276
writableDb.delete(NotificationTable.TABLE_NAME,
277-
NotificationTable.COLUMN_NAME_CREATED_TIME + " < " + ((System.currentTimeMillis() / 1000L) - 604800L),
277+
NotificationTable.COLUMN_NAME_CREATED_TIME + " < " + ((System.currentTimeMillis() / 1_000L) - 604800L),
278278
null);
279279
}
280280

0 commit comments

Comments
 (0)