Skip to content

Commit

Permalink
move unused service initialization out of main thread
Browse files Browse the repository at this point in the history
  • Loading branch information
jinliu9508 committed Aug 2, 2024
1 parent 429c1c9 commit e18e4b6
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 66 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import com.onesignal.core.internal.preferences.impl.PreferencesService
import com.onesignal.core.internal.purchases.impl.TrackAmazonPurchase
import com.onesignal.core.internal.purchases.impl.TrackGooglePurchase
import com.onesignal.core.internal.startup.IStartableService
import com.onesignal.core.internal.startup.StartupService
import com.onesignal.core.internal.time.ITime
import com.onesignal.core.internal.time.impl.Time
import com.onesignal.inAppMessages.IInAppMessagesManager
Expand All @@ -54,7 +53,6 @@ internal class CoreModule : IModule {
builder.register<DeviceService>().provides<IDeviceService>()
builder.register<Time>().provides<ITime>()
builder.register<DatabaseProvider>().provides<IDatabaseProvider>()
builder.register<StartupService>().provides<StartupService>()
builder.register<InstallIdService>().provides<IInstallIdService>()

// Params (Config)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import com.onesignal.core.internal.permissions.IRequestPermissionService

internal class RequestPermissionService(
private val _application: IApplicationService,
) : Activity(), IRequestPermissionService {
) : IRequestPermissionService {
var waiting = false
var fallbackToSettings = false
var shouldShowRequestPermissionRationaleBeforeRequest = false
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
package com.onesignal.core.internal.startup

import com.onesignal.common.services.ServiceProvider

internal class StartupService(
private val _bootstrapServices: List<IBootstrapService>,
private val _startableServices: List<IStartableService>,
private val services: ServiceProvider,
) {
fun bootstrap() {
// now that we have the params initialized, start everything else up
for (bootstrapService in _bootstrapServices)
bootstrapService.bootstrap()
services.getAllServices<IBootstrapService>().forEach { it.bootstrap() }
}

fun start() {
// now that we have the params initialized, start everything else up
for (startableService in _startableServices)
startableService.start()
// schedule to start all startable services in a separate thread
fun scheduleStart() {
Thread {
services.getAllServices<IStartableService>().forEach { it.start() }
}.start()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -86,50 +86,56 @@ internal class OneSignalImp : IOneSignal, IServiceProvider {
override val debug: IDebugManager = DebugManager()
override val session: ISessionManager get() =
if (isInitialized) {
_session!!
services.getService()
} else {
throw Exception(
"Must call 'initWithContext' before use",
)
}
override val notifications: INotificationsManager get() =
if (isInitialized) {
_notifications!!
services.getService()
} else {
throw Exception(
"Must call 'initWithContext' before use",
)
}
override val location: ILocationManager get() =
if (isInitialized) {
_location!!
services.getService()
} else {
throw Exception(
"Must call 'initWithContext' before use",
)
}
override val inAppMessages: IInAppMessagesManager get() =
if (isInitialized) {
iam!!
services.getService()
} else {
throw Exception(
"Must call 'initWithContext' before use",
)
}
override val user: IUserManager get() =
if (isInitialized) {
services.getService()
} else {
throw Exception(
"Must call 'initWithContext' before use",
)
}
override val user: IUserManager get() = if (isInitialized) _user!! else throw Exception("Must call 'initWithContext' before use")

// Services required by this class
private var _user: IUserManager? = null
private var _session: ISessionManager? = null
private var iam: IInAppMessagesManager? = null
private var _location: ILocationManager? = null
private var _notifications: INotificationsManager? = null
private var operationRepo: IOperationRepo? = null
private var identityModelStore: IdentityModelStore? = null
private var propertiesModelStore: PropertiesModelStore? = null
private var subscriptionModelStore: SubscriptionModelStore? = null
private var startupService: StartupService? = null
private var preferencesService: IPreferencesService? = null
private val operationRepo: IOperationRepo
get() = services.getService()
private val identityModelStore: IdentityModelStore
get() = services.getService()
private val propertiesModelStore: PropertiesModelStore
get() = services.getService()
private val subscriptionModelStore: SubscriptionModelStore
get() = services.getService()
private val preferencesService: IPreferencesService
get() = services.getService()

// Other State
private val services: ServiceProvider
Expand Down Expand Up @@ -234,21 +240,10 @@ internal class OneSignalImp : IOneSignal, IServiceProvider {
configModel!!.disableGMSMissingPrompt = _disableGMSMissingPrompt!!
}

// "Inject" the services required by this main class
_location = services.getService()
_user = services.getService()
_session = services.getService()
iam = services.getService()
_notifications = services.getService()
operationRepo = services.getService()
propertiesModelStore = services.getService()
identityModelStore = services.getService()
subscriptionModelStore = services.getService()
preferencesService = services.getService()

// Instantiate and call the IStartableServices
startupService = services.getService()
startupService!!.bootstrap()
val startupService = StartupService(services)

// bootstrap services
startupService.bootstrap()

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

pushSubscriptionModel.sdk = OneSignalUtils.SDK_VERSION
pushSubscriptionModel.deviceOS = Build.VERSION.RELEASE
pushSubscriptionModel.carrier = DeviceUtils.getCarrierName(services.getService<IApplicationService>().appContext) ?: ""
pushSubscriptionModel.appVersion = AndroidUtils.getAppVersion(services.getService<IApplicationService>().appContext) ?: ""
pushSubscriptionModel.carrier = DeviceUtils.getCarrierName(
services.getService<IApplicationService>().appContext,
) ?: ""
pushSubscriptionModel.appVersion = AndroidUtils.getAppVersion(
services.getService<IApplicationService>().appContext,
) ?: ""

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

startupService!!.start()
// schedule service starts out of main thread
startupService.scheduleStart()

isInitialized = true

return true
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,24 +1,38 @@
package com.onesignal.core.internal.startup

import com.onesignal.common.services.ServiceBuilder
import com.onesignal.common.services.ServiceProvider
import com.onesignal.debug.LogLevel
import com.onesignal.debug.internal.logging.Logging
import io.kotest.assertions.throwables.shouldThrowUnit
import io.kotest.core.spec.style.FunSpec
import io.kotest.matchers.shouldBe
import io.mockk.coVerifyOrder
import io.mockk.every
import io.mockk.mockk
import io.mockk.spyk
import io.mockk.verify

class StartupServiceTests : FunSpec({
fun setupServiceProvider(
bootstrapServices: List<IBootstrapService>,
startableServices: List<IStartableService>,
): ServiceProvider {
val serviceBuilder = ServiceBuilder()
for (reg in bootstrapServices)
serviceBuilder.register(reg).provides<IBootstrapService>()
for (reg in startableServices)
serviceBuilder.register(reg).provides<IStartableService>()
return serviceBuilder.build()
}

beforeAny {
Logging.logLevel = LogLevel.NONE
}

test("bootstrap with no IBootstrapService dependencies is a no-op") {
// Given
val startupService = StartupService(listOf(), listOf())
val startupService = StartupService(setupServiceProvider(listOf(), listOf()))

// When
startupService.bootstrap()
Expand All @@ -31,7 +45,7 @@ class StartupServiceTests : FunSpec({
val mockBootstrapService1 = spyk<IBootstrapService>()
val mockBootstrapService2 = spyk<IBootstrapService>()

val startupService = StartupService(listOf(mockBootstrapService1, mockBootstrapService2), listOf())
val startupService = StartupService(setupServiceProvider(listOf(mockBootstrapService1, mockBootstrapService2), listOf()))

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

val startupService = StartupService(listOf(mockBootstrapService1, mockBootstrapService2), listOf())
val startupService = StartupService(setupServiceProvider(listOf(mockBootstrapService1, mockBootstrapService2), listOf()))

// When
val actualException =
Expand All @@ -63,40 +77,41 @@ class StartupServiceTests : FunSpec({
verify(exactly = 0) { mockBootstrapService2.bootstrap() }
}

test("startup will call all IStartableService dependencies successfully") {
test("startup will call all IStartableService dependencies successfully after a short delay") {
// Given
val mockStartupService1 = spyk<IStartableService>()
val mockStartupService2 = spyk<IStartableService>()

val startupService = StartupService(listOf(), listOf(mockStartupService1, mockStartupService2))
val startupService = StartupService(setupServiceProvider(listOf(), listOf(mockStartupService1, mockStartupService2)))

// When
startupService.start()
startupService.scheduleStart()

// Then
Thread.sleep(10)
verify(exactly = 1) { mockStartupService1.start() }
verify(exactly = 1) { mockStartupService2.start() }
}

test("startup will propagate exception when an IStartableService throws an exception") {
test("scheduleStart does not block main thread") {
// Given
val exception = Exception("SOMETHING BAD")

val mockStartableService1 = mockk<IStartableService>()
every { mockStartableService1.start() } throws exception
val mockStartableService1 = spyk<IStartableService>()
val mockStartableService2 = spyk<IStartableService>()
val mockStartableService3 = spyk<IStartableService>()

val startupService = StartupService(listOf(), listOf(mockStartableService1, mockStartableService2))
val startupService = StartupService(setupServiceProvider(listOf(), listOf(mockStartableService1, mockStartableService2)))

// When
val actualException =
shouldThrowUnit<Exception> {
startupService.start()
}
startupService.scheduleStart()
mockStartableService3.start()

// Then
actualException shouldBe exception
verify(exactly = 1) { mockStartableService1.start() }
verify(exactly = 0) { mockStartableService2.start() }
Thread.sleep(10)
coVerifyOrder {
// service3 will call start() first even though service1 and service2 are scheduled first
mockStartableService3.start()
mockStartableService1.start()
mockStartableService2.start()
}
}
})

0 comments on commit e18e4b6

Please sign in to comment.