diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/services/SyncJobService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/services/SyncJobService.kt index cc664818ac..055b269334 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/services/SyncJobService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/services/SyncJobService.kt @@ -36,31 +36,51 @@ import com.onesignal.debug.internal.logging.Logging class SyncJobService : JobService() { override fun onStartJob(jobParameters: JobParameters): Boolean { suspendifyOnIO { - // init OneSignal in background - if (!OneSignal.initWithContext(this)) { - jobFinished(jobParameters, false) - return@suspendifyOnIO - } + var reschedule = false - val backgroundService = OneSignal.getService() - backgroundService.runBackgroundServices() + try { + // Init OneSignal in background + if (!OneSignal.initWithContext(this)) { + return@suspendifyOnIO + } - Logging.debug("LollipopSyncRunnable:JobFinished needsJobReschedule: " + backgroundService.needsJobReschedule) + val backgroundService = OneSignal.getService() + backgroundService.runBackgroundServices() - // Reschedule if needed - val reschedule = backgroundService.needsJobReschedule - backgroundService.needsJobReschedule = false - jobFinished(jobParameters, reschedule) + Logging.debug("LollipopSyncRunnable:JobFinished needsJobReschedule: " + backgroundService.needsJobReschedule) + + // Reschedule if needed + reschedule = backgroundService.needsJobReschedule + backgroundService.needsJobReschedule = false + } finally { + // Always call jobFinished to finish the job; onStopJob will handle the case when init failed + 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() - 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() + 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 + } } } diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt index a63560d09a..93fa9e6b11 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt @@ -5,6 +5,7 @@ import com.onesignal.common.services.ServiceProvider 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.assertions.throwables.shouldThrowUnit import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.shouldBe @@ -109,13 +110,20 @@ class StartupServiceTests : FunSpec({ } // When - startupService.scheduleStart() - mockStartableService2.start() + val thread = + Thread { + startupService.scheduleStart() + mockStartableService2.start() + } + thread.start() // Then // service2 does not block even though service1 is blocked verify(exactly = 1) { mockStartableService2.start() } + + // unblock the trigger and wait for scheduled service to complete blockTrigger.complete(Unit) + awaitIO() verify { mockStartableService1.start() } } }) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/services/SyncJobServiceTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/services/SyncJobServiceTests.kt new file mode 100644 index 0000000000..8a09f9b847 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/services/SyncJobServiceTests.kt @@ -0,0 +1,175 @@ +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(relaxed = true) + val mockBackgroundManager = mockk(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() } 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 initWithContext failed") { + // Given + val syncJobService = mocks.syncJobService + val jobParameters = mocks.jobParameters + coEvery { OneSignal.initWithContext(any()) } returns false + + // When + syncJobService.onStartJob(jobParameters) + + // Then + 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() } 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() } + } +}) diff --git a/OneSignalSDK/onesignal/location/src/test/java/com/onesignal/location/internal/LocationManagerTests.kt b/OneSignalSDK/onesignal/location/src/test/java/com/onesignal/location/internal/LocationManagerTests.kt index 7cf1f9dd96..945b11b8eb 100644 --- a/OneSignalSDK/onesignal/location/src/test/java/com/onesignal/location/internal/LocationManagerTests.kt +++ b/OneSignalSDK/onesignal/location/src/test/java/com/onesignal/location/internal/LocationManagerTests.kt @@ -11,6 +11,7 @@ import com.onesignal.location.internal.common.LocationConstants import com.onesignal.location.internal.common.LocationUtils import com.onesignal.location.internal.controller.ILocationController import com.onesignal.location.internal.permissions.LocationPermissionController +import com.onesignal.mocks.IOMockHelper import com.onesignal.mocks.IOMockHelper.awaitIO import com.onesignal.mocks.MockHelper import io.kotest.core.spec.style.FunSpec @@ -84,6 +85,8 @@ private class Mocks { class LocationManagerTests : FunSpec({ + listener(IOMockHelper) + lateinit var mocks: Mocks beforeAny {