Skip to content

Commit 10d1507

Browse files
jinliu9508jinliu9508
authored
Merge pull request #2141 from OneSignal/rollback-init-optimization
Revert "Merge pull request #2127 from OneSignal/get-service-by-getter"
2 parents 6078d77 + c2db8c8 commit 10d1507

File tree

5 files changed

+67
-82
lines changed

5 files changed

+67
-82
lines changed

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

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

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ 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
3536
import com.onesignal.core.internal.time.ITime
3637
import com.onesignal.core.internal.time.impl.Time
3738
import com.onesignal.inAppMessages.IInAppMessagesManager
@@ -53,6 +54,7 @@ internal class CoreModule : IModule {
5354
builder.register<DeviceService>().provides<IDeviceService>()
5455
builder.register<Time>().provides<ITime>()
5556
builder.register<DatabaseProvider>().provides<IDatabaseProvider>()
57+
builder.register<StartupService>().provides<StartupService>()
5658
builder.register<InstallIdService>().provides<IInstallIdService>()
5759

5860
// Params (Config)

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

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,18 @@
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-
83
internal class StartupService(
9-
private val services: ServiceProvider,
4+
private val _bootstrapServices: List<IBootstrapService>,
5+
private val _startableServices: List<IStartableService>,
106
) {
11-
private val coroutineScope = CoroutineScope(newSingleThreadContext(name = "StartupService"))
12-
137
fun bootstrap() {
14-
services.getAllServices<IBootstrapService>().forEach { it.bootstrap() }
8+
// now that we have the params initialized, start everything else up
9+
for (bootstrapService in _bootstrapServices)
10+
bootstrapService.bootstrap()
1511
}
1612

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-
}
13+
fun start() {
14+
// now that we have the params initialized, start everything else up
15+
for (startableService in _startableServices)
16+
startableService.start()
2217
}
2318
}

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

Lines changed: 36 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -86,56 +86,50 @@ internal class OneSignalImp : IOneSignal, IServiceProvider {
8686
override val debug: IDebugManager = DebugManager()
8787
override val session: ISessionManager get() =
8888
if (isInitialized) {
89-
services.getService()
89+
_session!!
9090
} else {
9191
throw Exception(
9292
"Must call 'initWithContext' before use",
9393
)
9494
}
9595
override val notifications: INotificationsManager get() =
9696
if (isInitialized) {
97-
services.getService()
97+
_notifications!!
9898
} else {
9999
throw Exception(
100100
"Must call 'initWithContext' before use",
101101
)
102102
}
103103
override val location: ILocationManager get() =
104104
if (isInitialized) {
105-
services.getService()
105+
_location!!
106106
} else {
107107
throw Exception(
108108
"Must call 'initWithContext' before use",
109109
)
110110
}
111111
override val inAppMessages: IInAppMessagesManager get() =
112112
if (isInitialized) {
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()
113+
iam!!
122114
} else {
123115
throw Exception(
124116
"Must call 'initWithContext' before use",
125117
)
126118
}
119+
override val user: IUserManager get() = if (isInitialized) _user!! else throw Exception("Must call 'initWithContext' before use")
127120

128121
// Services required by this class
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()
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
139133

140134
// Other State
141135
private val services: ServiceProvider
@@ -240,10 +234,21 @@ internal class OneSignalImp : IOneSignal, IServiceProvider {
240234
configModel!!.disableGMSMissingPrompt = _disableGMSMissingPrompt!!
241235
}
242236

243-
val startupService = StartupService(services)
244-
245-
// bootstrap services
246-
startupService.bootstrap()
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()
247252

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

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

303304
configModel!!.pushSubscriptionId = legacyPlayerId
304305
subscriptionModelStore!!.add(
@@ -327,9 +328,10 @@ internal class OneSignalImp : IOneSignal, IServiceProvider {
327328
Logging.debug("initWithContext: using cached user ${identityModelStore!!.model.onesignalId}")
328329
}
329330

330-
// schedule service starts out of main thread
331-
startupService.scheduleStart()
331+
startupService!!.start()
332+
332333
isInitialized = true
334+
333335
return true
334336
}
335337
}

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

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

3-
import com.onesignal.common.services.ServiceBuilder
4-
import com.onesignal.common.services.ServiceProvider
53
import com.onesignal.debug.LogLevel
64
import com.onesignal.debug.internal.logging.Logging
75
import io.kotest.assertions.throwables.shouldThrowUnit
86
import io.kotest.core.spec.style.FunSpec
97
import io.kotest.matchers.shouldBe
10-
import io.mockk.coVerifyOrder
118
import io.mockk.every
129
import io.mockk.mockk
1310
import io.mockk.spyk
1411
import io.mockk.verify
1512

1613
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-
}
2814

2915
beforeAny {
3016
Logging.logLevel = LogLevel.NONE
3117
}
3218

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

3723
// When
3824
startupService.bootstrap()
@@ -45,7 +31,7 @@ class StartupServiceTests : FunSpec({
4531
val mockBootstrapService1 = spyk<IBootstrapService>()
4632
val mockBootstrapService2 = spyk<IBootstrapService>()
4733

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

5036
// When
5137
startupService.bootstrap()
@@ -63,7 +49,7 @@ class StartupServiceTests : FunSpec({
6349
every { mockBootstrapService1.bootstrap() } throws exception
6450
val mockBootstrapService2 = spyk<IBootstrapService>()
6551

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

6854
// When
6955
val actualException =
@@ -77,41 +63,40 @@ class StartupServiceTests : FunSpec({
7763
verify(exactly = 0) { mockBootstrapService2.bootstrap() }
7864
}
7965

80-
test("startup will call all IStartableService dependencies successfully after a short delay") {
66+
test("startup will call all IStartableService dependencies successfully") {
8167
// Given
8268
val mockStartupService1 = spyk<IStartableService>()
8369
val mockStartupService2 = spyk<IStartableService>()
8470

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

8773
// When
88-
startupService.scheduleStart()
74+
startupService.start()
8975

9076
// Then
91-
Thread.sleep(10)
9277
verify(exactly = 1) { mockStartupService1.start() }
9378
verify(exactly = 1) { mockStartupService2.start() }
9479
}
9580

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

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

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

10897
// Then
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-
}
98+
actualException shouldBe exception
99+
verify(exactly = 1) { mockStartableService1.start() }
100+
verify(exactly = 0) { mockStartableService2.start() }
116101
}
117102
})

0 commit comments

Comments
 (0)