Skip to content

Commit

Permalink
Exception fix on cold start from notification restoring
Browse files Browse the repository at this point in the history 
* Fixed null exception that can happen when the app is restoring notifications when cold started.
  • Loading branch information
@jkasten2
jkasten2 committed Jun 30, 2016
1 parent 364827f commit a11751e
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 19 deletions.
Binary file modified OneSignalSDK.jar
View file
Binary file not shown.
7 changes: 4 additions & 3 deletions OneSignalSDK/app/src/test/java/com/onesignal/OneSignalPackagePrivateHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,13 @@ public static Bundle createInternalPayloadBundle(Bundle bundle) {
}

public static void NotificationBundleProcessor_ProcessFromGCMIntentService(Context context, Bundle bundle, NotificationExtenderService.OverrideSettings overrideSettings) {
if (overrideSettings == null)
overrideSettings = new NotificationExtenderService.OverrideSettings();

NotificationBundleProcessor.ProcessFromGCMIntentService(context, createInternalPayloadBundle(bundle), overrideSettings);
}

public static void NotificationBundleProcessor_ProcessFromGCMIntentService_NoWrap(Context context, Bundle bundle, NotificationExtenderService.OverrideSettings overrideSettings) {
NotificationBundleProcessor.ProcessFromGCMIntentService(context, bundle, overrideSettings);
}

public static boolean GcmBroadcastReceiver_processBundle(Context context, Bundle bundle) {
return GcmBroadcastReceiver.processBundle(context, bundle);
}
Expand Down
40 changes: 26 additions & 14 deletions OneSignalSDK/app/src/test/java/com/test/onesignal/GenerateNotificationRunner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
import com.onesignal.NotificationExtenderService;
import com.onesignal.NotificationOpenedProcessor;
import com.onesignal.OSNotificationPayload;
import com.onesignal.OneSignal;
import com.onesignal.OneSignalDbHelper;
import com.onesignal.OneSignalPackagePrivateHelper;
import com.onesignal.ShadowBadgeCountUpdater;
Expand All @@ -70,9 +69,12 @@
import org.robolectric.util.ServiceController;

import java.util.Iterator;
import java.util.List;
import java.util.Map;

import static com.onesignal.OneSignalPackagePrivateHelper.NotificationBundleProcessor_ProcessFromGCMIntentService;
import static com.onesignal.OneSignalPackagePrivateHelper.NotificationBundleProcessor_ProcessFromGCMIntentService_NoWrap;
import static com.onesignal.OneSignalPackagePrivateHelper.createInternalPayloadBundle;

@Config(packageName = "com.onesignal.example",
constants = BuildConfig.class,
shadows = { ShadowRoboNotificationManager.class, ShadowOneSignalRestClient.class, ShadowBadgeCountUpdater.class },
Expand Down Expand Up @@ -129,23 +131,33 @@ private Intent createOpenIntent(Bundle bundle) {
public void shouldSetTitleCorrectly() throws Exception {
// Should use app's Title by default
Bundle bundle = getBaseNotifBundle();
OneSignalPackagePrivateHelper.NotificationBundleProcessor_ProcessFromGCMIntentService(blankActivity, bundle, null);
NotificationBundleProcessor_ProcessFromGCMIntentService(blankActivity, bundle, null);
Assert.assertEquals("UnitTestApp", ShadowRoboNotificationManager.lastNotif.getContentTitle());
Assert.assertEquals(1, ShadowBadgeCountUpdater.lastCount);

// Should allow title from GCM payload.
bundle = getBaseNotifBundle("UUID2");
bundle.putString("title", "title123");
OneSignalPackagePrivateHelper.NotificationBundleProcessor_ProcessFromGCMIntentService(blankActivity, bundle, null);
NotificationBundleProcessor_ProcessFromGCMIntentService(blankActivity, bundle, null);
Assert.assertEquals("title123", ShadowRoboNotificationManager.lastNotif.getContentTitle());
Assert.assertEquals(2, ShadowBadgeCountUpdater.lastCount);
}

@Test
public void shouldProcessRestore() throws Exception {
Bundle bundle = createInternalPayloadBundle(getBaseNotifBundle());
bundle.putInt("android_notif_id", 0);
bundle.putBoolean("restoring", true);

NotificationBundleProcessor_ProcessFromGCMIntentService_NoWrap(blankActivity, bundle, null);
Assert.assertEquals("UnitTestApp", ShadowRoboNotificationManager.lastNotif.getContentTitle());
}

@Test
public void shouldHandleBasicNotifications() throws Exception {
// Make sure the notification got posted and the content is correct.
Bundle bundle = getBaseNotifBundle();
OneSignalPackagePrivateHelper.NotificationBundleProcessor_ProcessFromGCMIntentService(blankActivity, bundle, null);
NotificationBundleProcessor_ProcessFromGCMIntentService(blankActivity, bundle, null);
Assert.assertEquals(notifMessage, ShadowRoboNotificationManager.lastNotif.getContentText());
Assert.assertEquals(1, ShadowBadgeCountUpdater.lastCount);

Expand All @@ -167,14 +179,14 @@ public void shouldHandleBasicNotifications() throws Exception {
int firstNotifId = cursor.getInt(1);

// Should not display a duplicate notification, count should still be 1
OneSignalPackagePrivateHelper.NotificationBundleProcessor_ProcessFromGCMIntentService(blankActivity, bundle, null);
NotificationBundleProcessor_ProcessFromGCMIntentService(blankActivity, bundle, null);
cursor = readableDb.query(NotificationTable.TABLE_NAME, null, null, null, null, null, null);
Assert.assertEquals(1, cursor.getCount());
Assert.assertEquals(0, ShadowBadgeCountUpdater.lastCount);

// Display a second notification
bundle = getBaseNotifBundle("UUID2");
OneSignalPackagePrivateHelper.NotificationBundleProcessor_ProcessFromGCMIntentService(blankActivity, bundle, null);
NotificationBundleProcessor_ProcessFromGCMIntentService(blankActivity, bundle, null);
cursor = readableDb.query(NotificationTable.TABLE_NAME, new String[] { "android_notification_id" }, "android_notification_id <> " + firstNotifId, null, null, null, null);
cursor.moveToFirst();
int secondNotifId = cursor.getInt(0);
Expand All @@ -186,7 +198,7 @@ public void shouldHandleBasicNotifications() throws Exception {
// Should of been added for a total of 2 records now.
// First opened should of been cleaned up, 1 week old non opened notification should stay, and one new record.
bundle = getBaseNotifBundle("UUID3");
OneSignalPackagePrivateHelper.NotificationBundleProcessor_ProcessFromGCMIntentService(blankActivity, bundle, null);
NotificationBundleProcessor_ProcessFromGCMIntentService(blankActivity, bundle, null);
cursor = readableDb.query(NotificationTable.TABLE_NAME, new String[] { "android_notification_id" }, null, null, null, null, null);
Assert.assertEquals(2, cursor.getCount());
Assert.assertEquals(2, ShadowBadgeCountUpdater.lastCount);
Expand All @@ -208,7 +220,7 @@ public void shouldGenerate2BasicGroupNotifications() throws Exception {
// Make sure the notification got posted and the content is correct.
Bundle bundle = getBaseNotifBundle();
bundle.putString("grp", "test1");
OneSignalPackagePrivateHelper.NotificationBundleProcessor_ProcessFromGCMIntentService(blankActivity, bundle, null);
NotificationBundleProcessor_ProcessFromGCMIntentService(blankActivity, bundle, null);

Map<Integer, PostedNotification> postedNotifs = ShadowRoboNotificationManager.notifications;
Assert.assertEquals(2, postedNotifs.size());
Expand Down Expand Up @@ -240,7 +252,7 @@ public void shouldGenerate2BasicGroupNotifications() throws Exception {
bundle.putString("alert", "Notif test 2");
bundle.putString("custom", "{\"i\": \"UUID2\"}");
bundle.putString("grp", "test1");
OneSignalPackagePrivateHelper.NotificationBundleProcessor_ProcessFromGCMIntentService(blankActivity, bundle, null);
NotificationBundleProcessor_ProcessFromGCMIntentService(blankActivity, bundle, null);

postedNotifs = ShadowRoboNotificationManager.notifications;
Assert.assertEquals(2, postedNotifs.size());
Expand Down Expand Up @@ -275,7 +287,7 @@ public void shouldGenerate2BasicGroupNotifications() throws Exception {
bundle.putString("alert", "Notif test 3");
bundle.putString("custom", "{\"i\": \"UUID3\"}");
bundle.putString("grp", "test1");
OneSignalPackagePrivateHelper.NotificationBundleProcessor_ProcessFromGCMIntentService(blankActivity, bundle, null);
NotificationBundleProcessor_ProcessFromGCMIntentService(blankActivity, bundle, null);

postedNotifsIterator = postedNotifs.entrySet().iterator();
postedNotification = postedNotifsIterator.next().getValue();
Expand Down Expand Up @@ -336,7 +348,7 @@ public void shouldFireNotificationExtenderService() throws Exception {
ServiceController<NotificationExtenderServiceTest> controller = Robolectric.buildService(NotificationExtenderServiceTest.class);
NotificationExtenderServiceTest service = controller.attach().create().get();
Intent testIntent = new Intent(RuntimeEnvironment.application, NotificationExtenderServiceTest.class);
testIntent.putExtras(OneSignalPackagePrivateHelper.createInternalPayloadBundle(getBundleWithAllOptionsSet()));
testIntent.putExtras(createInternalPayloadBundle(getBundleWithAllOptionsSet()));
controller.withIntent(testIntent).startCommand(0, 0);

OSNotificationPayload notification = service.notification;
Expand Down Expand Up @@ -372,15 +384,15 @@ public void shouldFireNotificationExtenderService() throws Exception {

// Test a basic notification without anything special.
testIntent = new Intent(RuntimeEnvironment.application, NotificationExtenderServiceTest.class);
testIntent.putExtras(OneSignalPackagePrivateHelper.createInternalPayloadBundle(getBaseNotifBundle()));
testIntent.putExtras(createInternalPayloadBundle(getBaseNotifBundle()));
controller.withIntent(testIntent).startCommand(0, 0);
Assert.assertFalse(ShadowOneSignal.messages.contains("Error assigning"));


// Test that a notification is still displayed if the developer's code in onNotificationProcessing throws an Exception.
NotificationExtenderServiceTest.throwInAppCode = true;
testIntent = new Intent(RuntimeEnvironment.application, NotificationExtenderServiceTest.class);
testIntent.putExtras(OneSignalPackagePrivateHelper.createInternalPayloadBundle(getBaseNotifBundle("NewUUID1")));
testIntent.putExtras(createInternalPayloadBundle(getBaseNotifBundle("NewUUID1")));
controller.withIntent(testIntent).startCommand(0, 0);

Assert.assertTrue(ShadowOneSignal.messages.contains("onNotificationProcessing throw an exception"));
Expand Down
5 changes: 4 additions & 1 deletion OneSignalSDK/onesignal/src/main/java/com/onesignal/NotificationBundleProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,11 @@ static void ProcessFromGCMIntentService(Context context, Bundle bundle, Notifica
if (!restoring && OneSignal.notValidOrDuplicated(context, jsonPayload))
return;

if (bundle.containsKey("android_notif_id"))
if (bundle.containsKey("android_notif_id")) {
if (overrideSettings == null)
overrideSettings = new NotificationExtenderService.OverrideSettings();
overrideSettings.androidNotificationId = bundle.getInt("android_notif_id");
}

Process(context, restoring, jsonPayload, overrideSettings);
} catch (JSONException e) {
Expand Down
2 changes: 1 addition & 1 deletion OneSignalSDK/onesignal/src/main/java/com/onesignal/OneSignal.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ public void init() {
private static TrackGooglePurchase trackGooglePurchase;
private static TrackAmazonPurchase trackAmazonPurchase;

public static final String VERSION = "020500";
public static final String VERSION = "020501";

private static AdvertisingIdentifierProvider mainAdIdProvider = new AdvertisingIdProviderGPS();

Expand Down

5 comments on commit a11751e

@heoseok
Copy link

@heoseok heoseok commented on a11751e Jul 1, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

T^T
After I updated my application with current release, I've got many exception report today.

java.lang.NullPointerException
at com.onesignal.NotificationBundleProcessor.ProcessFromGCMIntentService(NotificationBundleProcessor.java:57)
at com.onesignal.GcmIntentService.onHandleIntent(GcmIntentService.java:54)
at android.app.IntentService$ServiceHandler.handleMessage(IntentService.java:65)
at android.os.Handler.dispatchMessage(Handler.java:102)
at android.os.Looper.loop(Looper.java:136)
at android.os.HandlerThread.run(HandlerThread.java:61)

@jkasten2
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@heoseok We released it yesterday a few hours after this commit. You can confirm you have 2.5.1 if OneSignal.VERSION is set to "020501".

You can try restarting Android Studio and re-syncing gradle.

@willwach
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@heoseok
Could please tell me, if the problems occur in Versions > 2.5.0?
We got this exception too in 2.5.0

@willwach
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jkasten2 Is there a way to reproduce this exception?

@jkasten2
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@willwach The error below was fixed in version 2.5.1. It would happen if the app process was not running at all (background or foreground) and a notification was received without a NotificationExtenderService setup.

java.lang.NullPointerException
at com.onesignal.NotificationBundleProcessor.ProcessFromGCMIntentService(NotificationBundleProcessor.java:57)

Add the full stacktrace here if you're seeing the issue still after this fix.

Please sign in to comment.