Fix: NPE in SyncJobService since 5.4

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/services/SyncJobService.kt

@@ -38,7 +38,6 @@ class SyncJobService : JobService() {
         suspendifyOnIO {
             // init OneSignal in background
             if (!OneSignal.initWithContext(this)) {
-                jobFinished(jobParameters, false)
                 return@suspendifyOnIO
             }
 
@@ -53,14 +52,29 @@ class SyncJobService : JobService() {
             jobFinished(jobParameters, reschedule)
         }
 
+        // Returning true means the job will always continue running and do everything else in IO thread
+        // When initWithContext failed, the background task will simply end
         return true
     }
 
     override fun onStopJob(jobParameters: JobParameters): Boolean {
-        // We assume init has been called via onStartJob
-        var backgroundService = OneSignal.getService<IBackgroundManager>()
-        val reschedule = backgroundService.cancelRunBackgroundServices()
-        Logging.debug("SyncJobService onStopJob called, system conditions not available reschedule: $reschedule")
-        return reschedule
+        /*
+         * After 5.4, onStartJob calls initWithContext in background. That introduced a small possibility
+         * when onStopJob is called before the initialization completes in the background. When that happens,
+         * OneSignal.getService will run into a NPE. In that case, we just need to omit the job and do not
+         * reschedule.
+         */
+
+        // Additional hardening in the event of getService failure
+        try {
+            // We assume init has been called via onStartJob\
+            val backgroundService = OneSignal.getService<IBackgroundManager>()
+            val reschedule = backgroundService.cancelRunBackgroundServices()
+            Logging.debug("SyncJobService onStopJob called, system conditions not available reschedule: $reschedule")
+            return reschedule
+        } catch (e: Exception) {
+            Logging.error("SyncJobService onStopJob failed, omit and do not reschedule")
+            return false
+        }
     }
 }

OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/services/SyncJobServiceTests.kt

@@ -0,0 +1,162 @@
+package com.onesignal.core.services
+
+import android.app.job.JobParameters
+import com.onesignal.OneSignal
+import com.onesignal.core.internal.background.IBackgroundManager
+import com.onesignal.debug.LogLevel
+import com.onesignal.debug.internal.logging.Logging
+import com.onesignal.mocks.IOMockHelper
+import com.onesignal.mocks.IOMockHelper.awaitIO
+import io.kotest.core.spec.style.FunSpec
+import io.kotest.matchers.shouldBe
+import io.mockk.coEvery
+import io.mockk.coVerify
+import io.mockk.every
+import io.mockk.mockk
+import io.mockk.mockkObject
+import io.mockk.spyk
+import io.mockk.unmockkAll
+import io.mockk.verify
+
+private class Mocks {
+    val syncJobService = spyk(SyncJobService(), recordPrivateCalls = true)
+    val jobParameters = mockk<JobParameters>(relaxed = true)
+    val mockBackgroundManager = mockk<IBackgroundManager>(relaxed = true)
+}
+
+class SyncJobServiceTests : FunSpec({
+    lateinit var mocks: Mocks
+
+    listener(IOMockHelper)
+
+    beforeAny {
+        Logging.logLevel = LogLevel.NONE
+        mocks = Mocks() // fresh instance for each test
+        mockkObject(OneSignal)
+        every { OneSignal.getService<IBackgroundManager>() } returns mocks.mockBackgroundManager
+    }
+
+    afterAny {
+        unmockkAll()
+    }
+
+    test("onStartJob returns true when initWithContext fails") {
+        // Given
+        val syncJobService = mocks.syncJobService
+        val jobParameters = mocks.jobParameters
+        coEvery { OneSignal.initWithContext(any()) } returns false
+
+        // When
+        val result = syncJobService.onStartJob(jobParameters)
+
+        // Then
+        result shouldBe true
+    }
+
+    test("onStartJob calls runBackgroundServices when initWithContext succeeds") {
+        // Given
+        val mockBackgroundManager = mocks.mockBackgroundManager
+        val syncJobService = mocks.syncJobService
+        val jobParameters = mocks.jobParameters
+        coEvery { OneSignal.initWithContext(any()) } returns true
+        every { mockBackgroundManager.needsJobReschedule } returns false
+
+        // When
+        val result = syncJobService.onStartJob(jobParameters)
+        awaitIO()
+
+        // Then
+        result shouldBe true
+        coVerify { mockBackgroundManager.runBackgroundServices() }
+        verify { syncJobService.jobFinished(jobParameters, false) }
+    }
+
+    test("onStartJob calls jobFinished with false when needsJobReschedule is false") {
+        // Given
+        val syncJobService = mocks.syncJobService
+        val jobParameters = mocks.jobParameters
+        coEvery { OneSignal.initWithContext(any()) } returns true
+        every { mocks.mockBackgroundManager.needsJobReschedule } returns false
+
+        // When
+        syncJobService.onStartJob(jobParameters)
+        awaitIO()
+
+        // Then
+        verify { syncJobService.jobFinished(jobParameters, false) }
+    }
+
+    test("onStartJob calls jobFinished with true when needsJobReschedule is true") {
+        // Given
+        val mockBackgroundManager = mocks.mockBackgroundManager
+        val syncJobService = mocks.syncJobService
+        val jobParameters = mocks.jobParameters
+        coEvery { OneSignal.initWithContext(any()) } returns true
+        every { mockBackgroundManager.needsJobReschedule } returns true
+
+        // When
+        syncJobService.onStartJob(jobParameters)
+        awaitIO()
+
+        // Then
+        verify { syncJobService.jobFinished(jobParameters, true) }
+        verify { mockBackgroundManager.needsJobReschedule = false }
+    }
+
+    test("onStartJob resets needsJobReschedule to false after reading it") {
+        // Given
+        val mockBackgroundManager = mocks.mockBackgroundManager
+        val syncJobService = mocks.syncJobService
+        val jobParameters = mocks.jobParameters
+        coEvery { OneSignal.initWithContext(any()) } returns true
+        every { mockBackgroundManager.needsJobReschedule } returns true
+
+        // When
+        syncJobService.onStartJob(jobParameters)
+        awaitIO()
+
+        // Then
+        verify { mockBackgroundManager.needsJobReschedule = false }
+    }
+
+    test("onStopJob returns false when OneSignal.getService throws") {
+        // Given
+        val syncJobService = mocks.syncJobService
+        val jobParameters = mocks.jobParameters
+        coEvery { OneSignal.getService<Any>() } throws NullPointerException()
+
+        // When
+        val result = syncJobService.onStopJob(jobParameters)
+
+        // Then
+        result shouldBe false
+    }
+
+    test("onStopJob calls cancelRunBackgroundServices and returns its result") {
+        // Given
+        val mockBackgroundManager = mocks.mockBackgroundManager
+        val syncJobService = mocks.syncJobService
+        val jobParameters = mocks.jobParameters
+        every { mockBackgroundManager.cancelRunBackgroundServices() } returns true
+
+        // When
+        val result = syncJobService.onStopJob(jobParameters)
+
+        // Then
+        result shouldBe true
+        verify { mockBackgroundManager.cancelRunBackgroundServices() }
+    }
+
+    test("onStopJob returns false when cancelRunBackgroundServices returns false") {
+        // Given
+        val mockBackgroundManager = mocks.mockBackgroundManager
+        every { mockBackgroundManager.cancelRunBackgroundServices() } returns false
+
+        // When
+        val result = mocks.syncJobService.onStopJob(mocks.jobParameters)
+
+        // Then
+        result shouldBe false
+        verify { mockBackgroundManager.cancelRunBackgroundServices() }
+    }
+})