Skip to content

Commit 1bd49af

Browse files
jinliu9508jinliu9508
authored
Fix: NPE in SyncJobService since 5.4 (#2500)
1 parent a2818ff commit 1bd49af

File tree

4 files changed

+225
-19
lines changed

4 files changed

+225
-19
lines changed

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

Lines changed: 37 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -36,31 +36,51 @@ import com.onesignal.debug.internal.logging.Logging
3636
class SyncJobService : JobService() {
3737
override fun onStartJob(jobParameters: JobParameters): Boolean {
3838
suspendifyOnIO {
39-
// init OneSignal in background
40-
if (!OneSignal.initWithContext(this)) {
41-
jobFinished(jobParameters, false)
42-
return@suspendifyOnIO
43-
}
39+
var reschedule = false
4440

45-
val backgroundService = OneSignal.getService<IBackgroundManager>()
46-
backgroundService.runBackgroundServices()
41+
try {
42+
// Init OneSignal in background
43+
if (!OneSignal.initWithContext(this)) {
44+
return@suspendifyOnIO
45+
}
4746

48-
Logging.debug("LollipopSyncRunnable:JobFinished needsJobReschedule: " + backgroundService.needsJobReschedule)
47+
val backgroundService = OneSignal.getService<IBackgroundManager>()
48+
backgroundService.runBackgroundServices()
4949

50-
// Reschedule if needed
51-
val reschedule = backgroundService.needsJobReschedule
52-
backgroundService.needsJobReschedule = false
53-
jobFinished(jobParameters, reschedule)
50+
Logging.debug("LollipopSyncRunnable:JobFinished needsJobReschedule: " + backgroundService.needsJobReschedule)
51+
52+
// Reschedule if needed
53+
reschedule = backgroundService.needsJobReschedule
54+
backgroundService.needsJobReschedule = false
55+
} finally {
56+
// Always call jobFinished to finish the job; onStopJob will handle the case when init failed
57+
jobFinished(jobParameters, reschedule)
58+
}
5459
}
5560

61+
// Returning true means the job will always continue running and do everything else in IO thread
62+
// When initWithContext failed, the background task will simply end
5663
return true
5764
}
5865

5966
override fun onStopJob(jobParameters: JobParameters): Boolean {
60-
// We assume init has been called via onStartJob
61-
var backgroundService = OneSignal.getService<IBackgroundManager>()
62-
val reschedule = backgroundService.cancelRunBackgroundServices()
63-
Logging.debug("SyncJobService onStopJob called, system conditions not available reschedule: $reschedule")
64-
return reschedule
67+
/*
68+
* After 5.4, onStartJob calls initWithContext in background. That introduced a small possibility
69+
* when onStopJob is called before the initialization completes in the background. When that happens,
70+
* OneSignal.getService will run into a NPE. In that case, we just need to omit the job and do not
71+
* reschedule.
72+
*/
73+
74+
// Additional hardening in the event of getService failure
75+
try {
76+
// We assume init has been called via onStartJob\
77+
val backgroundService = OneSignal.getService<IBackgroundManager>()
78+
val reschedule = backgroundService.cancelRunBackgroundServices()
79+
Logging.debug("SyncJobService onStopJob called, system conditions not available reschedule: $reschedule")
80+
return reschedule
81+
} catch (e: Exception) {
82+
Logging.error("SyncJobService onStopJob failed, omit and do not reschedule")
83+
return false
84+
}
6585
}
6686
}

‎OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/startup/StartupServiceTests.kt‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import com.onesignal.common.services.ServiceProvider
55
import com.onesignal.debug.LogLevel
66
import com.onesignal.debug.internal.logging.Logging
77
import com.onesignal.mocks.IOMockHelper
8+
import com.onesignal.mocks.IOMockHelper.awaitIO
89
import io.kotest.assertions.throwables.shouldThrowUnit
910
import io.kotest.core.spec.style.FunSpec
1011
import io.kotest.matchers.shouldBe
@@ -109,13 +110,20 @@ class StartupServiceTests : FunSpec({
109110
}
110111

111112
// When
112-
startupService.scheduleStart()
113-
mockStartableService2.start()
113+
val thread =
114+
Thread {
115+
startupService.scheduleStart()
116+
mockStartableService2.start()
117+
}
118+
thread.start()
114119

115120
// Then
116121
// service2 does not block even though service1 is blocked
117122
verify(exactly = 1) { mockStartableService2.start() }
123+
124+
// unblock the trigger and wait for scheduled service to complete
118125
blockTrigger.complete(Unit)
126+
awaitIO()
119127
verify { mockStartableService1.start() }
120128
}
121129
})

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

Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
package com.onesignal.core.services
2+
3+
import android.app.job.JobParameters
4+
import com.onesignal.OneSignal
5+
import com.onesignal.core.internal.background.IBackgroundManager
6+
import com.onesignal.debug.LogLevel
7+
import com.onesignal.debug.internal.logging.Logging
8+
import com.onesignal.mocks.IOMockHelper
9+
import com.onesignal.mocks.IOMockHelper.awaitIO
10+
import io.kotest.core.spec.style.FunSpec
11+
import io.kotest.matchers.shouldBe
12+
import io.mockk.coEvery
13+
import io.mockk.coVerify
14+
import io.mockk.every
15+
import io.mockk.mockk
16+
import io.mockk.mockkObject
17+
import io.mockk.spyk
18+
import io.mockk.unmockkAll
19+
import io.mockk.verify
20+
21+
private class Mocks {
22+
val syncJobService = spyk(SyncJobService(), recordPrivateCalls = true)
23+
val jobParameters = mockk<JobParameters>(relaxed = true)
24+
val mockBackgroundManager = mockk<IBackgroundManager>(relaxed = true)
25+
}
26+
27+
class SyncJobServiceTests : FunSpec({
28+
lateinit var mocks: Mocks
29+
30+
listener(IOMockHelper)
31+
32+
beforeAny {
33+
Logging.logLevel = LogLevel.NONE
34+
mocks = Mocks() // fresh instance for each test
35+
mockkObject(OneSignal)
36+
every { OneSignal.getService<IBackgroundManager>() } returns mocks.mockBackgroundManager
37+
}
38+
39+
afterAny {
40+
unmockkAll()
41+
}
42+
43+
test("onStartJob returns true when initWithContext fails") {
44+
// Given
45+
val syncJobService = mocks.syncJobService
46+
val jobParameters = mocks.jobParameters
47+
coEvery { OneSignal.initWithContext(any()) } returns false
48+
49+
// When
50+
val result = syncJobService.onStartJob(jobParameters)
51+
52+
// Then
53+
result shouldBe true
54+
}
55+
56+
test("onStartJob calls runBackgroundServices when initWithContext succeeds") {
57+
// Given
58+
val mockBackgroundManager = mocks.mockBackgroundManager
59+
val syncJobService = mocks.syncJobService
60+
val jobParameters = mocks.jobParameters
61+
coEvery { OneSignal.initWithContext(any()) } returns true
62+
every { mockBackgroundManager.needsJobReschedule } returns false
63+
64+
// When
65+
val result = syncJobService.onStartJob(jobParameters)
66+
awaitIO()
67+
68+
// Then
69+
result shouldBe true
70+
coVerify { mockBackgroundManager.runBackgroundServices() }
71+
verify { syncJobService.jobFinished(jobParameters, false) }
72+
}
73+
74+
test("onStartJob calls jobFinished with false when initWithContext failed") {
75+
// Given
76+
val syncJobService = mocks.syncJobService
77+
val jobParameters = mocks.jobParameters
78+
coEvery { OneSignal.initWithContext(any()) } returns false
79+
80+
// When
81+
syncJobService.onStartJob(jobParameters)
82+
83+
// Then
84+
verify { syncJobService.jobFinished(jobParameters, false) }
85+
}
86+
87+
test("onStartJob calls jobFinished with false when needsJobReschedule is false") {
88+
// Given
89+
val syncJobService = mocks.syncJobService
90+
val jobParameters = mocks.jobParameters
91+
coEvery { OneSignal.initWithContext(any()) } returns true
92+
every { mocks.mockBackgroundManager.needsJobReschedule } returns false
93+
94+
// When
95+
syncJobService.onStartJob(jobParameters)
96+
awaitIO()
97+
98+
// Then
99+
verify { syncJobService.jobFinished(jobParameters, false) }
100+
}
101+
102+
test("onStartJob calls jobFinished with true when needsJobReschedule is true") {
103+
// Given
104+
val mockBackgroundManager = mocks.mockBackgroundManager
105+
val syncJobService = mocks.syncJobService
106+
val jobParameters = mocks.jobParameters
107+
coEvery { OneSignal.initWithContext(any()) } returns true
108+
every { mockBackgroundManager.needsJobReschedule } returns true
109+
110+
// When
111+
syncJobService.onStartJob(jobParameters)
112+
awaitIO()
113+
114+
// Then
115+
verify { syncJobService.jobFinished(jobParameters, true) }
116+
verify { mockBackgroundManager.needsJobReschedule = false }
117+
}
118+
119+
test("onStartJob resets needsJobReschedule to false after reading it") {
120+
// Given
121+
val mockBackgroundManager = mocks.mockBackgroundManager
122+
val syncJobService = mocks.syncJobService
123+
val jobParameters = mocks.jobParameters
124+
coEvery { OneSignal.initWithContext(any()) } returns true
125+
every { mockBackgroundManager.needsJobReschedule } returns true
126+
127+
// When
128+
syncJobService.onStartJob(jobParameters)
129+
awaitIO()
130+
131+
// Then
132+
verify { mockBackgroundManager.needsJobReschedule = false }
133+
}
134+
135+
test("onStopJob returns false when OneSignal.getService throws") {
136+
// Given
137+
val syncJobService = mocks.syncJobService
138+
val jobParameters = mocks.jobParameters
139+
coEvery { OneSignal.getService<Any>() } throws NullPointerException()
140+
141+
// When
142+
val result = syncJobService.onStopJob(jobParameters)
143+
144+
// Then
145+
result shouldBe false
146+
}
147+
148+
test("onStopJob calls cancelRunBackgroundServices and returns its result") {
149+
// Given
150+
val mockBackgroundManager = mocks.mockBackgroundManager
151+
val syncJobService = mocks.syncJobService
152+
val jobParameters = mocks.jobParameters
153+
every { mockBackgroundManager.cancelRunBackgroundServices() } returns true
154+
155+
// When
156+
val result = syncJobService.onStopJob(jobParameters)
157+
158+
// Then
159+
result shouldBe true
160+
verify { mockBackgroundManager.cancelRunBackgroundServices() }
161+
}
162+
163+
test("onStopJob returns false when cancelRunBackgroundServices returns false") {
164+
// Given
165+
val mockBackgroundManager = mocks.mockBackgroundManager
166+
every { mockBackgroundManager.cancelRunBackgroundServices() } returns false
167+
168+
// When
169+
val result = mocks.syncJobService.onStopJob(mocks.jobParameters)
170+
171+
// Then
172+
result shouldBe false
173+
verify { mockBackgroundManager.cancelRunBackgroundServices() }
174+
}
175+
})

‎OneSignalSDK/onesignal/location/src/test/java/com/onesignal/location/internal/LocationManagerTests.kt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import com.onesignal.location.internal.common.LocationConstants
1111
import com.onesignal.location.internal.common.LocationUtils
1212
import com.onesignal.location.internal.controller.ILocationController
1313
import com.onesignal.location.internal.permissions.LocationPermissionController
14+
import com.onesignal.mocks.IOMockHelper
1415
import com.onesignal.mocks.IOMockHelper.awaitIO
1516
import com.onesignal.mocks.MockHelper
1617
import io.kotest.core.spec.style.FunSpec
@@ -84,6 +85,8 @@ private class Mocks {
8485

8586
class LocationManagerTests : FunSpec({
8687

88+
listener(IOMockHelper)
89+
8790
lateinit var mocks: Mocks
8891

8992
beforeAny {

0 commit comments

Comments
 (0)