Skip to content

Commit 08c0f30

Browse files
jinliu9508jinliu9508
authored
Merge pull request #2127 from OneSignal/get-service-by-getter
Move start call of unused services off the main thread
2 parents e9433ad + 1346a32 commit 08c0f30

File tree

5 files changed

+82
-67
lines changed

5 files changed

+82
-67
lines changed

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@ class ServiceRegistrationReflection<T>(
5656
Logging.debug("${ServiceProvider.indent}Already instantiated: $obj")
5757
return obj
5858
}
59-
6059
// use reflection to try to instantiate the thing
6160
for (constructor in clazz.constructors) {
6261
if (doesHaveAllParameters(constructor, provider)) {

‎OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/CoreModule.kt‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ import com.onesignal.core.internal.preferences.impl.PreferencesService
3232
import com.onesignal.core.internal.purchases.impl.TrackAmazonPurchase
3333
import com.onesignal.core.internal.purchases.impl.TrackGooglePurchase
3434
import com.onesignal.core.internal.startup.IStartableService
35-
import com.onesignal.core.internal.startup.StartupService
3635
import com.onesignal.core.internal.time.ITime
3736
import com.onesignal.core.internal.time.impl.Time
3837
import com.onesignal.inAppMessages.IInAppMessagesManager
@@ -54,7 +53,6 @@ internal class CoreModule : IModule {
5453
builder.register<DeviceService>().provides<IDeviceService>()
5554
builder.register<Time>().provides<ITime>()
5655
builder.register<DatabaseProvider>().provides<IDatabaseProvider>()
57-
builder.register<StartupService>().provides<StartupService>()
5856
builder.register<InstallIdService>().provides<IInstallIdService>()
5957

6058
// Params (Config)

‎OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/startup/StartupService.kt‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,23 @@
11
package com.onesignal.core.internal.startup
22

3+
import com.onesignal.common.services.ServiceProvider
4+
import kotlinx.coroutines.CoroutineScope
5+
import kotlinx.coroutines.launch
6+
import kotlinx.coroutines.newSingleThreadContext
7+
38
internal class StartupService(
4-
private val _bootstrapServices: List<IBootstrapService>,
5-
private val _startableServices: List<IStartableService>,
9+
private val services: ServiceProvider,
610
) {
11+
private val coroutineScope = CoroutineScope(newSingleThreadContext(name = "StartupService"))
12+
713
fun bootstrap() {
8-
// now that we have the params initialized, start everything else up
9-
for (bootstrapService in _bootstrapServices)
10-
bootstrapService.bootstrap()
14+
services.getAllServices<IBootstrapService>().forEach { it.bootstrap() }
1115
}
1216

13-
fun start() {
14-
// now that we have the params initialized, start everything else up
15-
for (startableService in _startableServices)
16-
startableService.start()
17+
// schedule to start all startable services in a separate thread
18+
fun scheduleStart() {
19+
coroutineScope.launch {
20+
services.getAllServices<IStartableService>().forEach { it.start() }
21+
}
1722
}
1823
}

‎OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt‎

Lines changed: 34 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -86,50 +86,56 @@ internal class OneSignalImp : IOneSignal, IServiceProvider {
8686
override val debug: IDebugManager = DebugManager()
8787
override val session: ISessionManager get() =
8888
if (isInitialized) {
89-
_session!!
89+
services.getService()
9090
} else {
9191
throw Exception(
9292
"Must call 'initWithContext' before use",
9393
)
9494
}
9595
override val notifications: INotificationsManager get() =
9696
if (isInitialized) {
97-
_notifications!!
97+
services.getService()
9898
} else {
9999
throw Exception(
100100
"Must call 'initWithContext' before use",
101101
)
102102
}
103103
override val location: ILocationManager get() =
104104
if (isInitialized) {
105-
_location!!
105+
services.getService()
106106
} else {
107107
throw Exception(
108108
"Must call 'initWithContext' before use",
109109
)
110110
}
111111
override val inAppMessages: IInAppMessagesManager get() =
112112
if (isInitialized) {
113-
iam!!
113+
services.getService()
114+
} else {
115+
throw Exception(
116+
"Must call 'initWithContext' before use",
117+
)
118+
}
119+
override val user: IUserManager get() =
120+
if (isInitialized) {
121+
services.getService()
114122
} else {
115123
throw Exception(
116124
"Must call 'initWithContext' before use",
117125
)
118126
}
119-
override val user: IUserManager get() = if (isInitialized) _user!! else throw Exception("Must call 'initWithContext' before use")
120127

121128
// Services required by this class
122-
private var _user: IUserManager? = null
123-
private var _session: ISessionManager? = null
124-
private var iam: IInAppMessagesManager? = null
125-
private var _location: ILocationManager? = null
126-
private var _notifications: INotificationsManager? = null
127-
private var operationRepo: IOperationRepo? = null
128-
private var identityModelStore: IdentityModelStore? = null
129-
private var propertiesModelStore: PropertiesModelStore? = null
130-
private var subscriptionModelStore: SubscriptionModelStore? = null
131-
private var startupService: StartupService? = null
132-
private var preferencesService: IPreferencesService? = null
129+
private val operationRepo: IOperationRepo
130+
get() = services.getService()
131+
private val identityModelStore: IdentityModelStore
132+
get() = services.getService()
133+
private val propertiesModelStore: PropertiesModelStore
134+
get() = services.getService()
135+
private val subscriptionModelStore: SubscriptionModelStore
136+
get() = services.getService()
137+
private val preferencesService: IPreferencesService
138+
get() = services.getService()
133139

134140
// Other State
135141
private val services: ServiceProvider
@@ -234,21 +240,10 @@ internal class OneSignalImp : IOneSignal, IServiceProvider {
234240
configModel!!.disableGMSMissingPrompt = _disableGMSMissingPrompt!!
235241
}
236242

237-
// "Inject" the services required by this main class
238-
_location = services.getService()
239-
_user = services.getService()
240-
_session = services.getService()
241-
iam = services.getService()
242-
_notifications = services.getService()
243-
operationRepo = services.getService()
244-
propertiesModelStore = services.getService()
245-
identityModelStore = services.getService()
246-
subscriptionModelStore = services.getService()
247-
preferencesService = services.getService()
248-
249-
// Instantiate and call the IStartableServices
250-
startupService = services.getService()
251-
startupService!!.bootstrap()
243+
val startupService = StartupService(services)
244+
245+
// bootstrap services
246+
startupService.bootstrap()
252247

253248
if (forceCreateUser || !identityModelStore!!.model.hasProperty(IdentityConstants.ONESIGNAL_ID)) {
254249
val legacyPlayerId =
@@ -298,8 +293,12 @@ internal class OneSignalImp : IOneSignal, IServiceProvider {
298293

299294
pushSubscriptionModel.sdk = OneSignalUtils.SDK_VERSION
300295
pushSubscriptionModel.deviceOS = Build.VERSION.RELEASE
301-
pushSubscriptionModel.carrier = DeviceUtils.getCarrierName(services.getService<IApplicationService>().appContext) ?: ""
302-
pushSubscriptionModel.appVersion = AndroidUtils.getAppVersion(services.getService<IApplicationService>().appContext) ?: ""
296+
pushSubscriptionModel.carrier = DeviceUtils.getCarrierName(
297+
services.getService<IApplicationService>().appContext,
298+
) ?: ""
299+
pushSubscriptionModel.appVersion = AndroidUtils.getAppVersion(
300+
services.getService<IApplicationService>().appContext,
301+
) ?: ""
303302

304303
configModel!!.pushSubscriptionId = legacyPlayerId
305304
subscriptionModelStore!!.add(
@@ -328,10 +327,9 @@ internal class OneSignalImp : IOneSignal, IServiceProvider {
328327
Logging.debug("initWithContext: using cached user ${identityModelStore!!.model.onesignalId}")
329328
}
330329

331-
startupService!!.start()
332-
330+
// schedule service starts out of main thread
331+
startupService.scheduleStart()
333332
isInitialized = true
334-
335333
return true
336334
}
337335
}

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

Lines changed: 34 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,38 @@
11
package com.onesignal.core.internal.startup
22

3+
import com.onesignal.common.services.ServiceBuilder
4+
import com.onesignal.common.services.ServiceProvider
35
import com.onesignal.debug.LogLevel
46
import com.onesignal.debug.internal.logging.Logging
57
import io.kotest.assertions.throwables.shouldThrowUnit
68
import io.kotest.core.spec.style.FunSpec
79
import io.kotest.matchers.shouldBe
10+
import io.mockk.coVerifyOrder
811
import io.mockk.every
912
import io.mockk.mockk
1013
import io.mockk.spyk
1114
import io.mockk.verify
1215

1316
class StartupServiceTests : FunSpec({
17+
fun setupServiceProvider(
18+
bootstrapServices: List<IBootstrapService>,
19+
startableServices: List<IStartableService>,
20+
): ServiceProvider {
21+
val serviceBuilder = ServiceBuilder()
22+
for (reg in bootstrapServices)
23+
serviceBuilder.register(reg).provides<IBootstrapService>()
24+
for (reg in startableServices)
25+
serviceBuilder.register(reg).provides<IStartableService>()
26+
return serviceBuilder.build()
27+
}
1428

1529
beforeAny {
1630
Logging.logLevel = LogLevel.NONE
1731
}
1832

1933
test("bootstrap with no IBootstrapService dependencies is a no-op") {
2034
// Given
21-
val startupService = StartupService(listOf(), listOf())
35+
val startupService = StartupService(setupServiceProvider(listOf(), listOf()))
2236

2337
// When
2438
startupService.bootstrap()
@@ -31,7 +45,7 @@ class StartupServiceTests : FunSpec({
3145
val mockBootstrapService1 = spyk<IBootstrapService>()
3246
val mockBootstrapService2 = spyk<IBootstrapService>()
3347

34-
val startupService = StartupService(listOf(mockBootstrapService1, mockBootstrapService2), listOf())
48+
val startupService = StartupService(setupServiceProvider(listOf(mockBootstrapService1, mockBootstrapService2), listOf()))
3549

3650
// When
3751
startupService.bootstrap()
@@ -49,7 +63,7 @@ class StartupServiceTests : FunSpec({
4963
every { mockBootstrapService1.bootstrap() } throws exception
5064
val mockBootstrapService2 = spyk<IBootstrapService>()
5165

52-
val startupService = StartupService(listOf(mockBootstrapService1, mockBootstrapService2), listOf())
66+
val startupService = StartupService(setupServiceProvider(listOf(mockBootstrapService1, mockBootstrapService2), listOf()))
5367

5468
// When
5569
val actualException =
@@ -63,40 +77,41 @@ class StartupServiceTests : FunSpec({
6377
verify(exactly = 0) { mockBootstrapService2.bootstrap() }
6478
}
6579

66-
test("startup will call all IStartableService dependencies successfully") {
80+
test("startup will call all IStartableService dependencies successfully after a short delay") {
6781
// Given
6882
val mockStartupService1 = spyk<IStartableService>()
6983
val mockStartupService2 = spyk<IStartableService>()
7084

71-
val startupService = StartupService(listOf(), listOf(mockStartupService1, mockStartupService2))
85+
val startupService = StartupService(setupServiceProvider(listOf(), listOf(mockStartupService1, mockStartupService2)))
7286

7387
// When
74-
startupService.start()
88+
startupService.scheduleStart()
7589

7690
// Then
91+
Thread.sleep(10)
7792
verify(exactly = 1) { mockStartupService1.start() }
7893
verify(exactly = 1) { mockStartupService2.start() }
7994
}
8095

81-
test("startup will propagate exception when an IStartableService throws an exception") {
96+
test("scheduleStart does not block main thread") {
8297
// Given
83-
val exception = Exception("SOMETHING BAD")
84-
85-
val mockStartableService1 = mockk<IStartableService>()
86-
every { mockStartableService1.start() } throws exception
98+
val mockStartableService1 = spyk<IStartableService>()
8799
val mockStartableService2 = spyk<IStartableService>()
100+
val mockStartableService3 = spyk<IStartableService>()
88101

89-
val startupService = StartupService(listOf(), listOf(mockStartableService1, mockStartableService2))
102+
val startupService = StartupService(setupServiceProvider(listOf(), listOf(mockStartableService1, mockStartableService2)))
90103

91104
// When
92-
val actualException =
93-
shouldThrowUnit<Exception> {
94-
startupService.start()
95-
}
105+
startupService.scheduleStart()
106+
mockStartableService3.start()
96107

97108
// Then
98-
actualException shouldBe exception
99-
verify(exactly = 1) { mockStartableService1.start() }
100-
verify(exactly = 0) { mockStartableService2.start() }
109+
Thread.sleep(10)
110+
coVerifyOrder {
111+
// service3 will call start() first even though service1 and service2 are scheduled first
112+
mockStartableService3.start()
113+
mockStartableService1.start()
114+
mockStartableService2.start()
115+
}
101116
}
102117
})

0 commit comments

Comments
 (0)