Skip to content

Commit 640e1e2

Browse files
jinliu9508jinliu9508
committed
Fix: NPE in SyncJobService since 5.4
1 parent 6f2d264 commit 640e1e2

File tree

2 files changed

+183
-6
lines changed

2 files changed

+183
-6
lines changed

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

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,13 @@ import com.onesignal.OneSignal
3232
import com.onesignal.common.threading.suspendifyOnIO
3333
import com.onesignal.core.internal.background.IBackgroundManager
3434
import com.onesignal.debug.internal.logging.Logging
35+
import kotlinx.coroutines.runBlocking
3536

3637
class SyncJobService : JobService() {
3738
override fun onStartJob(jobParameters: JobParameters): Boolean {
3839
suspendifyOnIO {
3940
// init OneSignal in background
4041
if (!OneSignal.initWithContext(this)) {
41-
jobFinished(jobParameters, false)
4242
return@suspendifyOnIO
4343
}
4444

@@ -53,14 +53,29 @@ class SyncJobService : JobService() {
5353
jobFinished(jobParameters, reschedule)
5454
}
5555

56+
// Returning true means the job will always continue running and do everything else in IO thread
57+
// When initWithContext failed, the background task will simply end
5658
return true
5759
}
5860

5961
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
62+
/*
63+
* After 5.4, onStartJob calls initWithContext in background. That introduced a small possibility
64+
* when onStopJob is called before the initialization completes in the background. When that happens,
65+
* OneSignal.getService will run into a NPE. In that case, we just need to omit the job and do not
66+
* reschedule.
67+
*/
68+
69+
// Additional hardening in the event of getService failure
70+
try {
71+
// We assume init has been called via onStartJob\
72+
val backgroundService = OneSignal.getService<IBackgroundManager>()
73+
val reschedule = backgroundService.cancelRunBackgroundServices()
74+
Logging.debug("SyncJobService onStopJob called, system conditions not available reschedule: $reschedule")
75+
return reschedule
76+
} catch (e: Exception) {
77+
Logging.error("SyncJobService onStopJob failed, omit and do not reschedule")
78+
return false
79+
}
6580
}
6681
}

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

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
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 needsJobReschedule is false") {
75+
// Given
76+
val syncJobService = mocks.syncJobService
77+
val jobParameters = mocks.jobParameters
78+
coEvery { OneSignal.initWithContext(any()) } returns true
79+
every { mocks.mockBackgroundManager.needsJobReschedule } returns false
80+
81+
// When
82+
syncJobService.onStartJob(jobParameters)
83+
awaitIO()
84+
85+
// Then
86+
verify { syncJobService.jobFinished(jobParameters, false) }
87+
}
88+
89+
test("onStartJob calls jobFinished with true when needsJobReschedule is true") {
90+
// Given
91+
val mockBackgroundManager = mocks.mockBackgroundManager
92+
val syncJobService = mocks.syncJobService
93+
val jobParameters = mocks.jobParameters
94+
coEvery { OneSignal.initWithContext(any()) } returns true
95+
every { mockBackgroundManager.needsJobReschedule } returns true
96+
97+
// When
98+
syncJobService.onStartJob(jobParameters)
99+
awaitIO()
100+
101+
// Then
102+
verify { syncJobService.jobFinished(jobParameters, true) }
103+
verify { mockBackgroundManager.needsJobReschedule = false }
104+
}
105+
106+
test("onStartJob resets needsJobReschedule to false after reading it") {
107+
// Given
108+
val mockBackgroundManager = mocks.mockBackgroundManager
109+
val syncJobService = mocks.syncJobService
110+
val jobParameters = mocks.jobParameters
111+
coEvery { OneSignal.initWithContext(any()) } returns true
112+
every { mockBackgroundManager.needsJobReschedule } returns true
113+
114+
// When
115+
syncJobService.onStartJob(jobParameters)
116+
awaitIO()
117+
118+
// Then
119+
verify { mockBackgroundManager.needsJobReschedule = false }
120+
}
121+
122+
test("onStopJob returns false when OneSignal.getService throws") {
123+
// Given
124+
val syncJobService = mocks.syncJobService
125+
val jobParameters = mocks.jobParameters
126+
coEvery { OneSignal.getService<Any>() } throws NullPointerException()
127+
128+
// When
129+
val result = syncJobService.onStopJob(jobParameters)
130+
131+
// Then
132+
result shouldBe false
133+
}
134+
135+
test("onStopJob calls cancelRunBackgroundServices and returns its result") {
136+
// Given
137+
val mockBackgroundManager = mocks.mockBackgroundManager
138+
val syncJobService = mocks.syncJobService
139+
val jobParameters = mocks.jobParameters
140+
every { mockBackgroundManager.cancelRunBackgroundServices() } returns true
141+
142+
// When
143+
val result = syncJobService.onStopJob(jobParameters)
144+
145+
// Then
146+
result shouldBe true
147+
verify { mockBackgroundManager.cancelRunBackgroundServices() }
148+
}
149+
150+
test("onStopJob returns false when cancelRunBackgroundServices returns false") {
151+
// Given
152+
val mockBackgroundManager = mocks.mockBackgroundManager
153+
every { mockBackgroundManager.cancelRunBackgroundServices() } returns false
154+
155+
// When
156+
val result = mocks.syncJobService.onStopJob(mocks.jobParameters)
157+
158+
// Then
159+
result shouldBe false
160+
verify { mockBackgroundManager.cancelRunBackgroundServices() }
161+
}
162+
})

0 commit comments

Comments
 (0)