Skip to content

Commit

Permalink
Merge pull request #1689 from OneSignal/user-model/minor-cleanup
Browse files Browse the repository at this point in the history
[User Model] Minor cleanup
  • Loading branch information
brismithers authored and jinliu9508 committed Feb 6, 2024
2 parents fa9b3e9 + 61e2d94 commit 3ec72ee
Show file tree
Hide file tree
Showing 28 changed files with 179 additions and 168 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import com.onesignal.common.AndroidUtils
import com.onesignal.common.IDManager
import com.onesignal.common.JSONUtils
import com.onesignal.common.events.CallbackProducer
import com.onesignal.common.events.ICallbackProducer
import com.onesignal.common.exceptions.BackendException
import com.onesignal.common.modeling.ISingletonModelStoreChangeHandler
import com.onesignal.common.modeling.ModelChangedArgs
Expand Down Expand Up @@ -43,10 +42,8 @@ import com.onesignal.session.internal.session.ISessionService
import com.onesignal.user.IUserManager
import com.onesignal.user.internal.subscriptions.ISubscriptionChangedHandler
import com.onesignal.user.internal.subscriptions.ISubscriptionManager
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock
import kotlinx.coroutines.withContext

internal class InAppMessagesManager(
private val _applicationService: IApplicationService,
Expand Down Expand Up @@ -74,8 +71,8 @@ internal class InAppMessagesManager(
ITriggerHandler,
ISessionLifecycleHandler {

private val _lifecycleCallback: ICallbackProducer<IInAppMessageLifecycleHandler> = CallbackProducer()
private val _messageClickCallback: ICallbackProducer<IInAppMessageClickHandler> = CallbackProducer()
private val _lifecycleCallback = CallbackProducer<IInAppMessageLifecycleHandler>()
private val _messageClickCallback = CallbackProducer<IInAppMessageClickHandler>()

// IAMs loaded remotely from on_session
// If on_session won't be called this will be loaded from cache
Expand Down Expand Up @@ -376,7 +373,7 @@ internal class InAppMessagesManager(
Logging.verbose("IAMManager.messageWasDismissed: inAppMessageLifecycleHandler is null")
return
}
_lifecycleCallback.fire { it.onDidDismissInAppMessage(message) }
_lifecycleCallback.fireOnMain { it.onDidDismissInAppMessage(message) }

_state.inAppMessageIdShowing = null

Expand Down Expand Up @@ -475,15 +472,15 @@ internal class InAppMessagesManager(
Logging.verbose("IAMManager.onMessageWillDisplay: inAppMessageLifecycleHandler is null")
return
}
_lifecycleCallback.fire { it.onWillDisplayInAppMessage(message) }
_lifecycleCallback.fireOnMain { it.onWillDisplayInAppMessage(message) }
}

override fun onMessageWasDisplayed(message: InAppMessage) {
if (!_lifecycleCallback.hasCallback) {
Logging.verbose("IAMManager.onMessageWasDisplayed: inAppMessageLifecycleHandler is null")
return
}
_lifecycleCallback.fire { it.onDidDisplayInAppMessage(message) }
_lifecycleCallback.fireOnMain { it.onDidDisplayInAppMessage(message) }

if (message.isPreview) {
return
Expand Down Expand Up @@ -552,7 +549,7 @@ internal class InAppMessagesManager(
Logging.verbose("IAMManager.onMessageWillDismiss: inAppMessageLifecycleHandler is null")
return
}
_lifecycleCallback.fire { it.onWillDismissInAppMessage(message) }
_lifecycleCallback.fireOnMain { it.onWillDismissInAppMessage(message) }
}

override fun onMessageWasDismissed(message: InAppMessage) {
Expand Down Expand Up @@ -707,10 +704,7 @@ internal class InAppMessagesManager(
// Check that only on the handler
// Any outcome sent on this callback should count as DIRECT from this IAM
_influenceManager.onDirectInfluenceFromIAM(messageId)

withContext(Dispatchers.Main) {
_messageClickCallback.fire { it.inAppMessageClicked(action) }
}
_messageClickCallback.suspendingFireOnMain { it.inAppMessageClicked(action) }
}

private suspend fun fireRESTCallForPageChange(message: InAppMessage, page: InAppMessagePage) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ internal class InAppBackendService(
val json: JSONObject = object : JSONObject() {
init {
put("app_id", appId)
put("device_type", _deviceService.deviceType)
put("device_type", _deviceService.deviceType.value)
put("player_id", subscriptionId)
put("click_id", clickId)
put("variant_id", variantId)
Expand Down Expand Up @@ -129,7 +129,7 @@ internal class InAppBackendService(
put("app_id", appId)
put("player_id", subscriptionId)
put("variant_id", variantId)
put("device_type", _deviceService.deviceType)
put("device_type", _deviceService.deviceType.value)
put("page_id", pageId)
}
}
Expand All @@ -155,7 +155,7 @@ internal class InAppBackendService(
put("app_id", appId)
put("player_id", subscriptionId)
put("variant_id", variantId)
put("device_type", _deviceService.deviceType)
put("device_type", _deviceService.deviceType.value)
put("first_impression", true)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,9 @@ object MockHelper {
return mockLanguageContext
}

const val DEFAULT_DEVICE_TYPE = 1

fun deviceService(): IDeviceService {
val deviceService = mockk<IDeviceService>()
every { deviceService.deviceType } returns DEFAULT_DEVICE_TYPE
every { deviceService.deviceType } returns IDeviceService.DeviceType.Android
return deviceService
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package com.onesignal.notifications.internal
import android.app.Activity
import com.onesignal.common.events.EventProducer
import com.onesignal.common.exceptions.BackendException
import com.onesignal.common.threading.suspendifyOnMain
import com.onesignal.common.threading.suspendifyOnThread
import com.onesignal.core.internal.application.IApplicationLifecycleHandler
import com.onesignal.core.internal.application.IApplicationService
Expand Down Expand Up @@ -100,9 +99,7 @@ internal class NotificationsManager(

if (oldPermissionStatus != isEnabled) {
// switch over to the main thread for the firing of the event
suspendifyOnMain {
_permissionChangedNotifier.fire { it.onPermissionChanged(isEnabled) }
}
_permissionChangedNotifier.fireOnMain { it.onPermissionChanged(isEnabled) }
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.onesignal.notifications.internal.backend

import com.onesignal.common.exceptions.BackendException
import com.onesignal.core.internal.device.IDeviceService
import org.json.JSONObject

/**
Expand All @@ -18,7 +19,7 @@ internal interface INotificationBackendService {
* @param subscriptionId The specific subscription within the [appId] the notification has been received for.
* @param deviceType The type of device the notification was received at.
*/
suspend fun updateNotificationAsReceived(appId: String, notificationId: String, subscriptionId: String, deviceType: Int)
suspend fun updateNotificationAsReceived(appId: String, notificationId: String, subscriptionId: String, deviceType: IDeviceService.DeviceType)

/**
* Update the provided notification as opened by a specific subscription.
Expand All @@ -30,7 +31,7 @@ internal interface INotificationBackendService {
* @param subscriptionId The specific subscription within the [appId] the notification has been received for.
* @param deviceType The type of device the notification was received at.
*/
suspend fun updateNotificationAsOpened(appId: String, notificationId: String, subscriptionId: String, deviceType: Int)
suspend fun updateNotificationAsOpened(appId: String, notificationId: String, subscriptionId: String, deviceType: IDeviceService.DeviceType)

/**
* Send a notification using the provided payload.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.onesignal.notifications.internal.backend.impl

import com.onesignal.common.exceptions.BackendException
import com.onesignal.core.internal.device.IDeviceService
import com.onesignal.core.internal.http.IHttpClient
import com.onesignal.notifications.internal.backend.INotificationBackendService
import org.json.JSONObject
Expand All @@ -9,11 +10,11 @@ internal class NotificationBackendService(
private val _httpClient: IHttpClient
) : INotificationBackendService {

override suspend fun updateNotificationAsReceived(appId: String, notificationId: String, subscriptionId: String, deviceType: Int) {
override suspend fun updateNotificationAsReceived(appId: String, notificationId: String, subscriptionId: String, deviceType: IDeviceService.DeviceType) {
val jsonBody: JSONObject = JSONObject()
.put("app_id", appId)
.put("player_id", subscriptionId)
.put("device_type", deviceType)
.put("device_type", deviceType.value)

var response = _httpClient.put("notifications/$notificationId/report_received", jsonBody)

Expand All @@ -22,12 +23,12 @@ internal class NotificationBackendService(
}
}

override suspend fun updateNotificationAsOpened(appId: String, notificationId: String, subscriptionId: String, deviceType: Int) {
override suspend fun updateNotificationAsOpened(appId: String, notificationId: String, subscriptionId: String, deviceType: IDeviceService.DeviceType) {
val jsonBody = JSONObject()
jsonBody.put("app_id", appId)
jsonBody.put("player_id", subscriptionId)
jsonBody.put("opened", true)
jsonBody.put("device_type", deviceType)
jsonBody.put("device_type", deviceType.value)

var response = _httpClient.put("notifications/$notificationId", jsonBody)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ internal class NotificationLifecycleService(
if (_extOpenedCallback.hasCallback && _unprocessedOpenedNotifs.any()) {
for (data in _unprocessedOpenedNotifs) {
val openedResult = NotificationHelper.generateNotificationOpenedResult(data, _time)
_extOpenedCallback.fire { it.notificationOpened(openedResult) }
_extOpenedCallback.fireOnMain { it.notificationOpened(openedResult) }
}
}
}
Expand Down Expand Up @@ -76,7 +76,7 @@ internal class NotificationLifecycleService(
// we will immediately fire the handler.
if (_extOpenedCallback.hasCallback) {
val openResult = NotificationHelper.generateNotificationOpenedResult(data, _time)
_extOpenedCallback.fire { it.notificationOpened(openResult) }
_extOpenedCallback.fireOnMain { it.notificationOpened(openResult) }
} else {
_unprocessedOpenedNotifs.add(data)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
package com.onesignal.common.events

import com.onesignal.common.threading.suspendifyOnMain
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.withContext

/**
* A standard implementation that implements [ICallbackProducer] to make callbacks less burdensome
* to the user.
* A standard implementation that implements [ICallbackNotifier] and additional functionality to
* make callbacks less burdensome to the user.
*/
open class CallbackProducer<THandler>() : ICallbackProducer<THandler> {
open class CallbackProducer<THandler>() : ICallbackNotifier<THandler> {

private var _callback: THandler? = null

Expand All @@ -22,15 +26,55 @@ open class CallbackProducer<THandler>() : ICallbackProducer<THandler> {
*
* @param callback The callback will be invoked if one exists, allowing you to call the handler.
*/
override fun fire(callback: (THandler) -> Unit) {
fun fire(callback: (THandler) -> Unit) {
if (_callback != null) {
callback(_callback!!)
}
}

/**
* Call this to fire the callback which will allow the caller to drive the calling of the
* callback handler if one exists. It is done this way to avoid this abstract class from
* knowing the specific signature of the handler. The callback will be invoked asynchronously
* on the main thread. Control will be returned immediately, most likely prior to the callbacks
* being invoked.
*
* @param callback The callback will be invoked if one exists, allowing you to call the handler.
*/
fun fireOnMain(callback: (THandler) -> Unit) {
suspendifyOnMain {
if (_callback != null) {
callback(_callback!!)
}
}
}

/**
* Call this to fire the callback which will allow the caller to drive the calling of the
* callback handler if one exists. It is done this way to avoid this abstract class from
* knowing the specific signature of the handler.
*
* @param callback The callback will be invoked if one exists, allowing you to call the handler.
*/
suspend fun suspendingFire(callback: suspend (THandler) -> Unit) {
if (_callback != null) {
callback(_callback!!)
}
}

/**
* Call this to fire the callback which will allow the caller to drive the calling of the
* callback handler if one exists. It is done this way to avoid this abstract class from
* knowing the specific signature of the handler. The callback will be invoked on the main
* thread.
*
* @param callback The callback will be invoked if one exists, allowing you to call the handler.
*/
suspend fun suspendingFireOnMain(callback: suspend (THandler) -> Unit) {
if (_callback != null) {
withContext(Dispatchers.Main) {
callback(_callback!!)
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
package com.onesignal.common.events

import com.onesignal.common.threading.suspendifyOnMain
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.withContext

/**
* A standard implementation that implements [IEventProducer] to make event firing less burdensome
* to the user.
* A standard implementation that implements [IEventNotifier] and additional functionality to make
* event firing less burdensome to the user.
*/
open class EventProducer<THandler> : IEventProducer<THandler> {
open class EventProducer<THandler> : IEventNotifier<THandler> {

private val _subscribers: MutableList<THandler> = mutableListOf()

Expand All @@ -16,12 +20,6 @@ open class EventProducer<THandler> : IEventProducer<THandler> {
_subscribers.remove(handler)
}

override fun fire(callback: (THandler) -> Unit) {
for (s in _subscribers) {
callback(s)
}
}

/**
* Subscribe all from an existing producer to this subscriber.
*/
Expand All @@ -32,25 +30,55 @@ open class EventProducer<THandler> : IEventProducer<THandler> {
}

/**
* Conditional fire all subscribers *until one indicates to stop firing*
*
* @param callback The callback, returns true when no more subscribers should be called.
* Call this to fire the callback which will allow the caller to drive the calling of the
* callback handlers if there are any.
*
* @return Whether a subscriber callback indicated to stop firing.
* @param callback The callback will be invoked for each subscribed handler, allowing you to call the handler.
*/
fun conditionalFire(callback: (THandler) -> Boolean): Boolean {
fun fire(callback: (THandler) -> Unit) {
for (s in _subscribers) {
if (!callback(s)) {
return true
}
callback(s)
}
}

return true
/**
* Call this to fire the callback which will allow the caller to drive the calling of the
* callback handlers if there are any. The callback will be invoked asynchronously on the main
* thread. Control will be returned immediately, most likely prior to the callbacks being invoked.
*
* @param callback The callback will be invoked for each subscribed handler, allowing you to call the handler.
*/
fun fireOnMain(callback: (THandler) -> Unit) {
suspendifyOnMain {
for (s in _subscribers) {
callback(s)
}
}
}

/**
* Call this to fire the callback which will allow the caller to drive the calling of the
* callback handlers if there are any.
*
* @param callback The callback will be invoked for each subscribed handler, allowing you to call the handler.
*/
suspend fun suspendingFire(callback: suspend (THandler) -> Unit) {
for (s in _subscribers) {
callback(s)
}
}

/**
* Call this to fire the callback which will allow the caller to drive the calling of the
* callback handlers if there are any. The callback will be invoked on the main thread.
*
* @param callback The callback will be invoked for each subscribed handler, allowing you to call the handler.
*/
suspend fun suspendingFireOnMain(callback: suspend (THandler) -> Unit) {
withContext(Dispatchers.Main) {
for (s in _subscribers) {
callback(s)
}
}
}
}
Loading

0 comments on commit 3ec72ee

Please sign in to comment.