Skip to content

Commit

Permalink
Merge pull request #1484 from OneSignal/fix/notification-processed-twice
Browse files Browse the repository at this point in the history 
Use two `Workers` to display notification and send receive receipt
  • Loading branch information
@nan-li
nan-li authored Nov 13, 2021
2 parents 1fcacef + d80b60b commit dda19da
Show file tree
Hide file tree
Showing 20 changed files with 143 additions and 290 deletions.
1 change: 0 additions & 1 deletion OneSignalSDK/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ buildscript {
targetSdkVersion: 30
]
androidGradlePluginVersion = '3.6.2'
androidConcurrentFutures = '1.1.0'
googleServicesGradlePluginVersion = '4.3.2'
huaweiAgconnectVersion = '1.2.1.301'
huaweiHMSPushVersion = '5.3.0.304'
Expand Down
2 changes: 0 additions & 2 deletions OneSignalSDK/onesignal/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,6 @@ dependencies {
// Required for GoogleApiAvailability
implementation 'com.google.android.gms:play-services-base:[17.0.0, 17.6.99]'

implementation("androidx.concurrent:concurrent-futures:$androidConcurrentFutures")

// firebase-messaging:18.0.0 is the last version before going to AndroidX
// firebase-messaging:19.0.0 is the first version using AndroidX
api 'com.google.firebase:firebase-messaging:[19.0.0, 22.0.99]'
Expand Down
21 changes: 4 additions & 17 deletions OneSignalSDK/onesignal/src/main/java/com/onesignal/NotificationBundleProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.annotation.WorkerThread;
import androidx.concurrent.futures.CallbackToFutureAdapter;
import androidx.work.ListenableWorker;

import com.onesignal.OneSignalDbContract.NotificationTable;

Expand Down Expand Up @@ -100,8 +98,7 @@ public void onResult(boolean result) {
jsonStrPayload,
shownTimeStamp,
isRestoring,
false,
true);
false);

// Delay to prevent CPU spikes.
// Normally more than one notification is restored at a time.
Expand Down Expand Up @@ -164,11 +161,6 @@ private static int processJobForDisplay(OSNotificationController notificationCon
String osNotificationId = OSNotificationFormatHelper.getOSNotificationIdFromJson(notificationController.getNotificationJob().getJsonPayload());
OSNotificationWorkManager.removeNotificationIdProcessed(osNotificationId);
OneSignal.handleNotificationReceived(notificationJob);
} else {
CallbackToFutureAdapter.Completer<ListenableWorker.Result> callbackCompleter = notificationJob.getCallbackCompleter();
OneSignal.Log(OneSignal.LOG_LEVEL.DEBUG, "Process notification restored or IAM with callback completer: " + callbackCompleter);
if (callbackCompleter != null)
callbackCompleter.set(ListenableWorker.Result.success());
}

return androidNotificationId;
Expand All @@ -183,22 +175,18 @@ private static boolean shouldDisplayNotification(OSNotificationGenerationJob not
*/
static void processNotification(OSNotificationGenerationJob notificationJob, boolean opened, boolean notificationDisplayed) {
saveNotification(notificationJob, opened);
CallbackToFutureAdapter.Completer<ListenableWorker.Result> callbackCompleter = notificationJob.getCallbackCompleter();

if (!notificationDisplayed) {
// Notification channel disable or not displayed
// save notification as dismissed to avoid user re-enabling channel and notification being displayed due to restore
markNotificationAsDismissed(notificationJob);

OneSignal.Log(OneSignal.LOG_LEVEL.DEBUG, "Process notification not displayed with callback completer: " + callbackCompleter);
if (callbackCompleter != null)
callbackCompleter.set(ListenableWorker.Result.success());
return;
}

// Logic for when the notification is displayed
String notificationId = notificationJob.getApiNotificationId();
OSReceiveReceiptController.getInstance().sendReceiveReceipt(callbackCompleter, notificationId);
Context context = notificationJob.getContext();
OSReceiveReceiptController.getInstance().beginEnqueueingWork(context, notificationId);
OneSignal.getSessionManager().onNotificationReceived(notificationId);
}

Expand Down Expand Up @@ -439,8 +427,7 @@ public void onResult(boolean result) {
jsonPayload.toString(),
timestamp,
isRestoring,
isHighPriority,
true);
isHighPriority);

bundleResult.setWorkManagerProcessing(true);
notificationProcessingCallback.onResult(true);
Expand Down
2 changes: 1 addition & 1 deletion OneSignalSDK/onesignal/src/main/java/com/onesignal/NotificationSummaryManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ private static void restoreSummary(Context context, String group) {
null
);

OSNotificationRestoreWorkManager.showNotificationsFromCursor(context, cursor, 0, true);
OSNotificationRestoreWorkManager.showNotificationsFromCursor(context, cursor, 0);
} catch (Throwable t) {
OneSignal.Log(OneSignal.LOG_LEVEL.ERROR, "Error restoring notification records! ", t);
} finally {
Expand Down
40 changes: 0 additions & 40 deletions OneSignalSDK/onesignal/src/main/java/com/onesignal/OSDelayTaskController.kt
View file

This file was deleted.

10 changes: 2 additions & 8 deletions OneSignalSDK/onesignal/src/main/java/com/onesignal/OSNotificationController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@
import android.content.Context;

import androidx.annotation.Nullable;
import androidx.concurrent.futures.CallbackToFutureAdapter;
import androidx.work.ListenableWorker;

import org.json.JSONObject;

Expand All @@ -46,7 +44,6 @@ public class OSNotificationController {
static final String GOOGLE_SENT_TIME_KEY = "google.sent_time";
static final String GOOGLE_TTL_KEY = "google.ttl";

private final CallbackToFutureAdapter.Completer<ListenableWorker.Result> callbackCompleter;
private final OSNotificationGenerationJob notificationJob;
private boolean restoring;
private boolean fromBackgroundLogic;
Expand All @@ -55,12 +52,9 @@ public class OSNotificationController {
this.restoring = restoring;
this.fromBackgroundLogic = fromBackgroundLogic;
this.notificationJob = notificationJob;
this.callbackCompleter = notificationJob.getCallbackCompleter();
}

OSNotificationController(CallbackToFutureAdapter.Completer<ListenableWorker.Result> callbackCompleter,
Context context, OSNotification notification, JSONObject jsonPayload, boolean restoring, boolean fromBackgroundLogic, Long timestamp) {
this.callbackCompleter = callbackCompleter;
OSNotificationController(Context context, OSNotification notification, JSONObject jsonPayload, boolean restoring, boolean fromBackgroundLogic, Long timestamp) {
this.restoring = restoring;
this.fromBackgroundLogic = fromBackgroundLogic;

Expand All @@ -74,7 +68,7 @@ public class OSNotificationController {
* @see OSNotificationGenerationJob
*/
private OSNotificationGenerationJob createNotificationJobFromCurrent(Context context, OSNotification notification, JSONObject jsonPayload, Long timestamp) {
OSNotificationGenerationJob notificationJob = new OSNotificationGenerationJob(callbackCompleter, context);
OSNotificationGenerationJob notificationJob = new OSNotificationGenerationJob(context);
notificationJob.setJsonPayload(jsonPayload);
notificationJob.setShownTimeStamp(timestamp);
notificationJob.setRestoring(restoring);
Expand Down
15 changes: 0 additions & 15 deletions OneSignalSDK/onesignal/src/main/java/com/onesignal/OSNotificationGenerationJob.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,12 @@
import android.content.Context;
import android.net.Uri;

import androidx.annotation.Nullable;
import androidx.concurrent.futures.CallbackToFutureAdapter;
import androidx.work.ListenableWorker;

import org.json.JSONObject;

import java.security.SecureRandom;

public class OSNotificationGenerationJob {

private CallbackToFutureAdapter.Completer<ListenableWorker.Result> callbackCompleter;
private OSNotification notification;
private Context context;
private JSONObject jsonPayload;
Expand All @@ -56,11 +51,6 @@ public class OSNotificationGenerationJob {
private Uri orgSound;

OSNotificationGenerationJob(Context context) {
this(null, context);
}

OSNotificationGenerationJob(CallbackToFutureAdapter.Completer<ListenableWorker.Result> callbackCompleter, Context context) {
this.callbackCompleter = callbackCompleter;
this.context = context;
}

Expand All @@ -74,11 +64,6 @@ public class OSNotificationGenerationJob {
this.notification = notification;
}

@Nullable
public CallbackToFutureAdapter.Completer<ListenableWorker.Result> getCallbackCompleter() {
return callbackCompleter;
}

/**
* Get the notification title from the payload
*/
Expand Down
7 changes: 3 additions & 4 deletions OneSignalSDK/onesignal/src/main/java/com/onesignal/OSNotificationRestoreWorkManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ private static void queryAndRestoreNotificationsAndBadgeCount(
OneSignalDbContract.NotificationTable._ID + " DESC", // sort order, new to old
NotificationLimitManager.MAX_NUMBER_OF_NOTIFICATIONS_STR // limit
);
showNotificationsFromCursor(context, cursor, DELAY_BETWEEN_NOTIFICATION_RESTORES_MS, false);
showNotificationsFromCursor(context, cursor, DELAY_BETWEEN_NOTIFICATION_RESTORES_MS);
BadgeCountUpdater.update(dbHelper, context);
} catch (Throwable t) {
OneSignal.Log(OneSignal.LOG_LEVEL.ERROR, "Error restoring notification records! ", t);
Expand Down Expand Up @@ -144,7 +144,7 @@ private static void skipVisibleNotifications(Context context, StringBuilder dbQu
* @param cursor - Source cursor to generate notifications from
* @param delay - Delay to slow down process to ensure we don't spike CPU and I/O on the device
*/
static void showNotificationsFromCursor(Context context, Cursor cursor, int delay, boolean needsWorkerThread) {
static void showNotificationsFromCursor(Context context, Cursor cursor, int delay) {
if (!cursor.moveToFirst())
return;

Expand All @@ -161,8 +161,7 @@ static void showNotificationsFromCursor(Context context, Cursor cursor, int dela
fullData,
dateTime,
true,
false,
needsWorkerThread
false
);

if (delay > 0)
Expand Down
79 changes: 21 additions & 58 deletions OneSignalSDK/onesignal/src/main/java/com/onesignal/OSNotificationWorkManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,13 @@
import android.content.Context;

import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.concurrent.futures.CallbackToFutureAdapter;
import androidx.work.Data;
import androidx.work.ExistingWorkPolicy;
import androidx.work.ListenableWorker;
import androidx.work.OneTimeWorkRequest;
import androidx.work.WorkManager;
import androidx.work.Worker;
import androidx.work.WorkerParameters;

import com.google.common.util.concurrent.ListenableFuture;

import org.json.JSONException;
import org.json.JSONObject;

Expand All @@ -23,7 +19,6 @@

class OSNotificationWorkManager {

private static final String OS_NOTIFICATION_ID = "os_bnotification_id";
private static final String ANDROID_NOTIF_ID_WORKER_DATA_PARAM = "android_notif_id";
private static final String JSON_PAYLOAD_WORKER_DATA_PARAM = "json_payload";
private static final String TIMESTAMP_WORKER_DATA_PARAM = "timestamp";
Expand Down Expand Up @@ -54,20 +49,9 @@ static void removeNotificationIdProcessed(String osNotificationId) {
}

static void beginEnqueueingWork(Context context, String osNotificationId, int androidNotificationId, String jsonPayload, long timestamp,
boolean isRestoring, boolean isHighPriority, boolean needsWorkerThread) {
if (!needsWorkerThread) {
try {
JSONObject jsonPayloadObject = new JSONObject(jsonPayload);
processNotificationData(context, androidNotificationId, jsonPayloadObject, isRestoring, timestamp);
} catch (JSONException e) {
OneSignal.onesignalLog(OneSignal.LOG_LEVEL.ERROR, "Error occurred parsing jsonPayload to JSONObject in beginEnqueueingWork e: " + e.getMessage());
e.printStackTrace();
}
return;
}
boolean isRestoring, boolean isHighPriority) {
// TODO: Need to figure out how to implement the isHighPriority param
Data inputData = new Data.Builder()
.putString(OS_NOTIFICATION_ID, osNotificationId)
.putInt(ANDROID_NOTIF_ID_WORKER_DATA_PARAM, androidNotificationId)
.putString(JSON_PAYLOAD_WORKER_DATA_PARAM, jsonPayload)
.putLong(TIMESTAMP_WORKER_DATA_PARAM, timestamp)
Expand All @@ -83,29 +67,15 @@ static void beginEnqueueingWork(Context context, String osNotificationId, int an
.enqueueUniqueWork(osNotificationId, ExistingWorkPolicy.KEEP, workRequest);
}

public static class NotificationWorker extends ListenableWorker {
public static class NotificationWorker extends Worker {

public NotificationWorker(@NonNull Context context, @NonNull WorkerParameters workerParams) {
super(context, workerParams);
}

@NonNull
@Override
public ListenableFuture<Result> startWork() {
return CallbackToFutureAdapter.getFuture(new CallbackToFutureAdapter.Resolver<Result>() {
@Nullable
@Override
public Object attachCompleter(@NonNull CallbackToFutureAdapter.Completer<Result> completer) throws Exception {
String notificationId = NotificationWorker.this.doWork(completer);

// This value is used only for debug purposes: it will be used
// in toString() of returned future or error cases.
return "NotificationWorkerFutureCallback_" + notificationId;
}
});
}

private String doWork(@NonNull CallbackToFutureAdapter.Completer<Result> completer) {
public Result doWork() {
Data inputData = getInputData();
try {
OneSignal.onesignalLog(OneSignal.LOG_LEVEL.DEBUG, "NotificationWorker running doWork with data: " + inputData);
Expand All @@ -116,7 +86,6 @@ private String doWork(@NonNull CallbackToFutureAdapter.Completer<Result> complet
boolean isRestoring = inputData.getBoolean(IS_RESTORING_WORKER_DATA_PARAM, false);

processNotificationData(
completer,
getApplicationContext(),
androidNotificationId,
jsonPayload,
Expand All @@ -125,36 +94,30 @@ private String doWork(@NonNull CallbackToFutureAdapter.Completer<Result> complet
} catch (JSONException e) {
OneSignal.onesignalLog(OneSignal.LOG_LEVEL.ERROR, "Error occurred doing work for job with id: " + getId().toString());
e.printStackTrace();
completer.setException(e);
return Result.failure();
}
return inputData.getString(OS_NOTIFICATION_ID);
return Result.success();
}
}

static void processNotificationData(Context context, int androidNotificationId, JSONObject jsonPayload,
boolean isRestoring, Long timestamp) {
processNotificationData(null, context, androidNotificationId, jsonPayload, isRestoring, timestamp);
}
private void processNotificationData(Context context, int androidNotificationId, JSONObject jsonPayload,
boolean isRestoring, Long timestamp) {
OSNotification notification = new OSNotification(null, jsonPayload, androidNotificationId);
OSNotificationController controller = new OSNotificationController(context, notification, jsonPayload, isRestoring, true, timestamp);
OSNotificationReceivedEvent notificationReceived = new OSNotificationReceivedEvent(controller, notification);

static void processNotificationData(CallbackToFutureAdapter.Completer<ListenableWorker.Result> completer,
Context context, int androidNotificationId, JSONObject jsonPayload,
boolean isRestoring, Long timestamp) {
OSNotification notification = new OSNotification(null, jsonPayload, androidNotificationId);
OSNotificationController controller = new OSNotificationController(completer, context, notification, jsonPayload, isRestoring, true, timestamp);
OSNotificationReceivedEvent notificationReceived = new OSNotificationReceivedEvent(controller, notification);
if (OneSignal.remoteNotificationReceivedHandler != null)
try {
OneSignal.remoteNotificationReceivedHandler.remoteNotificationReceived(context, notificationReceived);
} catch (Throwable t) {
OneSignal.Log(OneSignal.LOG_LEVEL.ERROR, "remoteNotificationReceived throw an exception. Displaying normal OneSignal notification.", t);
notificationReceived.complete(notification);

if (OneSignal.remoteNotificationReceivedHandler != null)
try {
OneSignal.remoteNotificationReceivedHandler.remoteNotificationReceived(context, notificationReceived);
} catch (Throwable t) {
OneSignal.Log(OneSignal.LOG_LEVEL.ERROR, "remoteNotificationReceived throw an exception. Displaying normal OneSignal notification.", t);
throw t;
}
else {
OneSignal.Log(OneSignal.LOG_LEVEL.WARN, "remoteNotificationReceivedHandler not setup, displaying normal OneSignal notification");
notificationReceived.complete(notification);

throw t;
}
else {
OneSignal.Log(OneSignal.LOG_LEVEL.WARN, "remoteNotificationReceivedHandler not setup, displaying normal OneSignal notification");
notificationReceived.complete(notification);
}
}
}
Loading

0 comments on commit dda19da

Please sign in to comment.