-
Notifications
You must be signed in to change notification settings - Fork 34
SDK-324 - Updated way of handling notifications to not use AsyncTask #981
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
03fe92d
d085489
2a1037a
b6ece27
cec31e6
10c199b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,8 @@ | ||
| package com.iterable.iterableapi; | ||
|
|
||
| import android.content.Context; | ||
| import android.os.AsyncTask; | ||
| import android.os.Bundle; | ||
|
|
||
| import androidx.annotation.NonNull; | ||
|
|
||
| import com.google.android.gms.tasks.Tasks; | ||
|
|
@@ -11,6 +11,7 @@ | |
| import com.google.firebase.messaging.RemoteMessage; | ||
|
|
||
| import java.util.Map; | ||
| import java.util.UUID; | ||
| import java.util.concurrent.ExecutionException; | ||
|
|
||
| public class IterableFirebaseMessagingService extends FirebaseMessagingService { | ||
|
|
@@ -56,12 +57,17 @@ public static boolean handleMessageReceived(@NonNull Context context, @NonNull R | |
| return false; | ||
| } | ||
|
|
||
| if (!IterableNotificationHelper.isGhostPush(extras)) { | ||
| boolean isGhostPush = IterableNotificationHelper.isGhostPush(extras); | ||
|
|
||
| if (!isGhostPush) { | ||
| if (!IterableNotificationHelper.isEmptyBody(extras)) { | ||
| IterableLogger.d(TAG, "Iterable push received " + messageData); | ||
| IterableNotificationBuilder notificationBuilder = IterableNotificationHelper.createNotification( | ||
| context.getApplicationContext(), extras); | ||
| new IterableNotificationManager().execute(notificationBuilder); | ||
|
|
||
| if (IterableNotificationHelper.hasAttachmentUrl(extras)) { | ||
| enqueueNotificationWork(context, extras); | ||
| } else { | ||
| handleNow(context, extras); | ||
| } | ||
| } else { | ||
| IterableLogger.d(TAG, "Iterable OS notification push received"); | ||
| } | ||
|
|
@@ -105,9 +111,7 @@ public static String getFirebaseToken() { | |
| String registrationToken = null; | ||
| try { | ||
| registrationToken = Tasks.await(FirebaseMessaging.getInstance().getToken()); | ||
| } catch (ExecutionException e) { | ||
| IterableLogger.e(TAG, e.getLocalizedMessage()); | ||
| } catch (InterruptedException e) { | ||
| } catch (ExecutionException | InterruptedException e) { | ||
| IterableLogger.e(TAG, e.getLocalizedMessage()); | ||
| } catch (Exception e) { | ||
| IterableLogger.e(TAG, "Failed to fetch firebase token"); | ||
|
|
@@ -122,25 +126,66 @@ public static String getFirebaseToken() { | |
| * @return Boolean indicating whether the message is an Iterable ghost push or silent push | ||
| */ | ||
| public static boolean isGhostPush(RemoteMessage remoteMessage) { | ||
| Map<String, String> messageData = remoteMessage.getData(); | ||
| try { | ||
| Map<String, String> messageData = remoteMessage.getData(); | ||
|
|
||
| if (messageData.isEmpty()) { | ||
| return false; | ||
| } | ||
|
|
||
| if (messageData == null || messageData.isEmpty()) { | ||
| Bundle extras = IterableNotificationHelper.mapToBundle(messageData); | ||
| return IterableNotificationHelper.isGhostPush(extras); | ||
| } catch (Exception e) { | ||
| IterableLogger.e(TAG, e.getMessage()); | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| Bundle extras = IterableNotificationHelper.mapToBundle(messageData); | ||
| return IterableNotificationHelper.isGhostPush(extras); | ||
| private static void enqueueNotificationWork(@NonNull final Context context, @NonNull final Bundle extras) { | ||
| IterableNotificationWorkScheduler scheduler = new IterableNotificationWorkScheduler(context); | ||
|
|
||
| scheduler.scheduleNotificationWork( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How does the scheduler actually create the notification on device?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How does it know the format of how to render the notification since it doesn't call the IterableNotificationHelper.createNotification function?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you check IterableNotificationWorker.doWork() you can see we are building the notification there, the unused createNotification apparently is there for backwards compatibility since 2019, that actually should be deprecated in my opinion, we can use this PR to do that |
||
| extras, | ||
| new IterableNotificationWorkScheduler.SchedulerCallback() { | ||
| @Override | ||
| public void onScheduleSuccess(UUID workId) { | ||
| IterableLogger.d(TAG, "Notification work scheduled: " + workId); | ||
| } | ||
|
|
||
| @Override | ||
| public void onScheduleFailure(Exception exception, Bundle notificationData) { | ||
| IterableLogger.e(TAG, "Failed to schedule notification work, falling back", exception); | ||
| handleFallbackNotification(context, notificationData); | ||
| } | ||
| } | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| class IterableNotificationManager extends AsyncTask<IterableNotificationBuilder, Void, Void> { | ||
| private static void handleNow(@NonNull Context context, @NonNull Bundle extras) { | ||
| try { | ||
| IterableNotificationBuilder notificationBuilder = IterableNotificationHelper.createNotification( | ||
| context.getApplicationContext(), extras); | ||
| if (notificationBuilder != null) { | ||
| IterableNotificationHelper.postNotificationOnDevice(context, notificationBuilder); | ||
| } | ||
| } catch (Exception e) { | ||
| IterableLogger.e(TAG, "Failed to post notification directly", e); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| protected Void doInBackground(IterableNotificationBuilder... params) { | ||
| if (params != null && params[0] != null) { | ||
| IterableNotificationBuilder notificationBuilder = params[0]; | ||
| IterableNotificationHelper.postNotificationOnDevice(notificationBuilder.context, notificationBuilder); | ||
| private static void handleFallbackNotification(@NonNull Context context, @NonNull Bundle extras) { | ||
| try { | ||
| IterableNotificationBuilder notificationBuilder = IterableNotificationHelper.createNotification( | ||
| context.getApplicationContext(), extras); | ||
| if (notificationBuilder != null) { | ||
| IterableNotificationHelper.postNotificationOnDevice(context, notificationBuilder); | ||
| IterableLogger.d(TAG, "✓ FALLBACK succeeded - notification posted directly"); | ||
| } else { | ||
| IterableLogger.w(TAG, "✗ FALLBACK: Notification builder is null"); | ||
| } | ||
| } catch (Exception fallbackException) { | ||
| IterableLogger.e(TAG, "✗ CRITICAL: FALLBACK also failed!", fallbackException); | ||
| IterableLogger.e(TAG, "NOTIFICATION WILL NOT BE DISPLAYED"); | ||
| } | ||
| return null; | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| package com.iterable.iterableapi; | ||
|
|
||
| import android.content.Context; | ||
| import android.os.Bundle; | ||
|
|
||
| import androidx.annotation.NonNull; | ||
| import androidx.annotation.Nullable; | ||
| import androidx.annotation.VisibleForTesting; | ||
| import androidx.work.OneTimeWorkRequest; | ||
| import androidx.work.OutOfQuotaPolicy; | ||
| import androidx.work.WorkManager; | ||
|
|
||
| import java.util.UUID; | ||
|
|
||
| class IterableNotificationWorkScheduler { | ||
|
|
||
| private static final String TAG = "IterableNotificationWorkScheduler"; | ||
|
|
||
| private final Context context; | ||
| private final WorkManager workManager; | ||
|
|
||
| interface SchedulerCallback { | ||
| void onScheduleSuccess(UUID workId); | ||
| void onScheduleFailure(Exception exception, Bundle notificationData); | ||
| } | ||
|
|
||
| IterableNotificationWorkScheduler(@NonNull Context context) { | ||
| this(context, WorkManager.getInstance(context)); | ||
| } | ||
|
|
||
| @VisibleForTesting | ||
| IterableNotificationWorkScheduler(@NonNull Context context, @NonNull WorkManager workManager) { | ||
| this.context = context.getApplicationContext(); | ||
| this.workManager = workManager; | ||
| } | ||
|
|
||
| void scheduleNotificationWork( | ||
| @NonNull Bundle notificationData, | ||
| @Nullable SchedulerCallback callback) { | ||
|
|
||
| try { | ||
| androidx.work.Data inputData = IterableNotificationWorker.createInputData( | ||
| notificationData | ||
| ); | ||
|
|
||
| OneTimeWorkRequest workRequest = new OneTimeWorkRequest.Builder(IterableNotificationWorker.class) | ||
| .setInputData(inputData) | ||
| .setExpedited(OutOfQuotaPolicy.RUN_AS_NON_EXPEDITED_WORK_REQUEST) | ||
| .build(); | ||
|
|
||
| workManager.enqueue(workRequest); | ||
|
|
||
| UUID workId = workRequest.getId(); | ||
| IterableLogger.d(TAG, "Notification work scheduled: " + workId); | ||
|
|
||
| if (callback != null) { | ||
| callback.onScheduleSuccess(workId); | ||
| } | ||
|
|
||
| } catch (Exception e) { | ||
| IterableLogger.e(TAG, "Failed to schedule notification work", e); | ||
|
|
||
| if (callback != null) { | ||
| callback.onScheduleFailure(e, notificationData); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @VisibleForTesting | ||
| Context getContext() { | ||
| return context; | ||
| } | ||
|
|
||
| @VisibleForTesting | ||
| WorkManager getWorkManager() { | ||
| return workManager; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verified that only the image attachment needs to schedule the NotificationWork.
Sounds are set as part of the notification category so it doesn't need to be scheduled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah from what i checked we don't need to do the sound async, only images