From 669fa514961663ebf1df65435aa75d9b259b8718 Mon Sep 17 00:00:00 2001 From: Nan Date: Thu, 14 Sep 2023 22:46:20 -0700 Subject: [PATCH 1/8] Fix bug for generating notification's android notification ID * Bug: The private setter was generating a random android notification ID but this private setter is never called. Setters are not triggered in initialization. This resulted in the android notification ID always being zero for every notification. * The fix: Now we set the random ID in initialization, and make the notification property immutable (a val instead of var) as it is never re-set anyway. --- .../common/NotificationGenerationJob.kt | 22 ++++++------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/common/NotificationGenerationJob.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/common/NotificationGenerationJob.kt index c22821f9e..33d1e8f49 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/common/NotificationGenerationJob.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/common/NotificationGenerationJob.kt @@ -7,25 +7,17 @@ import org.json.JSONObject import java.security.SecureRandom class NotificationGenerationJob( - private var _notification: Notification, + inNotification: Notification, var jsonPayload: JSONObject, ) { - var notification: Notification - get() = _notification - private set(value) { - // If there is no android ID on the notification coming in, create one either - // copying from the previous one or generating a new one. - if (value != null && !value!!.hasNotificationId()) { - val curNotification = _notification - if (curNotification != null && curNotification.hasNotificationId()) { - value.androidNotificationId = curNotification.androidNotificationId - } else { - value.androidNotificationId = SecureRandom().nextInt() - } - } + val notification: Notification = inNotification.setAndroidNotificationId() - _notification = value + private fun Notification.setAndroidNotificationId() = this.also { + // If there is no android ID on the notification coming in, generate a new one. + if (it != null && !it.hasNotificationId()) { + it.androidNotificationId = SecureRandom().nextInt() } + } var isRestoring = false var isNotificationToDisplay = false From b4de3712c08de09587a9dce24de1019758a683f9 Mon Sep 17 00:00:00 2001 From: Nan Date: Fri, 15 Sep 2023 12:09:22 -0700 Subject: [PATCH 2/8] Differentiate login failure cases * The implementation of login with external_id has not changed * Add new ExecutionResult type of FAIL_CONFLICT to differentiate this case from general failure case of FAIL_NORETRY * Add logs for both type of responses, but still try to create the user on both failures (as before) --- .../core/internal/operations/IOperationExecutor.kt | 6 ++++++ .../core/internal/operations/impl/OperationRepo.kt | 1 + .../impl/executors/IdentityOperationExecutor.kt | 6 +++--- .../impl/executors/LoginUserOperationExecutor.kt | 11 +++++++++-- 4 files changed, 19 insertions(+), 5 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/IOperationExecutor.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/IOperationExecutor.kt index 8af45ce8a..e27c49421 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/IOperationExecutor.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/IOperationExecutor.kt @@ -68,4 +68,10 @@ enum class ExecutionResult { * retried if authorization can be achieved. */ FAIL_UNAUTHORIZED, + + /** + * Used in special login case. + * The operation failed due to a conflict and can be handled. + */ + FAIL_CONFLICT, } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepo.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepo.kt index 91eaeb30c..937fc037e 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepo.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepo.kt @@ -167,6 +167,7 @@ internal class OperationRepo( } ExecutionResult.FAIL_UNAUTHORIZED, // TODO: Need to provide callback for app to reset JWT. For now, fail with no retry. ExecutionResult.FAIL_NORETRY, + ExecutionResult.FAIL_CONFLICT, -> { Logging.error("Operation execution failed without retry: $operations") // on failure we remove the operation from the store and wake any waiters diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/IdentityOperationExecutor.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/IdentityOperationExecutor.kt index 51818ea4f..5d11f99fd 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/IdentityOperationExecutor.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/IdentityOperationExecutor.kt @@ -51,10 +51,10 @@ internal class IdentityOperationExecutor( return when (responseType) { NetworkUtils.ResponseStatusType.RETRYABLE -> ExecutionResponse(ExecutionResult.FAIL_RETRY) - NetworkUtils.ResponseStatusType.INVALID, - NetworkUtils.ResponseStatusType.CONFLICT, - -> + NetworkUtils.ResponseStatusType.INVALID -> ExecutionResponse(ExecutionResult.FAIL_NORETRY) + NetworkUtils.ResponseStatusType.CONFLICT -> + ExecutionResponse(ExecutionResult.FAIL_CONFLICT) NetworkUtils.ResponseStatusType.UNAUTHORIZED -> ExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED) NetworkUtils.ResponseStatusType.MISSING -> { diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/LoginUserOperationExecutor.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/LoginUserOperationExecutor.kt index 8fd531da2..849c88078 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/LoginUserOperationExecutor.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/LoginUserOperationExecutor.kt @@ -86,9 +86,16 @@ internal class LoginUserOperationExecutor( ExecutionResponse(ExecutionResult.SUCCESS_STARTING_ONLY, mapOf(loginUserOp.onesignalId to backendOneSignalId)) } + ExecutionResult.FAIL_CONFLICT -> { + // When the SetAliasOperation fails with conflict that *most likely* means the externalId provided + // is already associated to a user. This *expected* condition means we must create a user. + // We hardcode the response of "user-2" in the log to provide information to the SDK consumer + Logging.debug("LoginUserOperationExecutor now handling 409 response with \"code\": \"user-2\" by switching to user with \"external_id\": \"${loginUserOp.externalId}\"") + createUser(loginUserOp, operations) + } ExecutionResult.FAIL_NORETRY -> { - // When the SetAliasOperation fails without retry that *most likely* means the externalId provided - // is already associated to a user. This expected condition means we must create a user. + // Some other failure occurred, still try to recover by creating the user + Logging.error("LoginUserOperationExecutor encountered error. Attempt to recover by switching to user with \"external_id\": \"${loginUserOp.externalId}\"") createUser(loginUserOp, operations) } else -> ExecutionResponse(result.result) From 2e4b382340782f1546d7cdde42c8b9e3fe82059b Mon Sep 17 00:00:00 2001 From: Shepherd Date: Wed, 20 Sep 2023 15:29:57 -0400 Subject: [PATCH 3/8] Rename kotlin modules to be named after namespace --- OneSignalSDK/onesignal/core/build.gradle | 2 ++ OneSignalSDK/onesignal/in-app-messages/build.gradle | 2 ++ OneSignalSDK/onesignal/location/build.gradle | 2 ++ OneSignalSDK/onesignal/notifications/build.gradle | 2 ++ 4 files changed, 8 insertions(+) diff --git a/OneSignalSDK/onesignal/core/build.gradle b/OneSignalSDK/onesignal/core/build.gradle index 81ed88df4..bd1219c24 100644 --- a/OneSignalSDK/onesignal/core/build.gradle +++ b/OneSignalSDK/onesignal/core/build.gradle @@ -42,6 +42,8 @@ android { jvmTarget = '1.8' } namespace 'com.onesignal.core' + + kotlinOptions.freeCompilerArgs += ['-module-name', namespace] } tasks.withType(Test) { diff --git a/OneSignalSDK/onesignal/in-app-messages/build.gradle b/OneSignalSDK/onesignal/in-app-messages/build.gradle index 25350132d..35529ab73 100644 --- a/OneSignalSDK/onesignal/in-app-messages/build.gradle +++ b/OneSignalSDK/onesignal/in-app-messages/build.gradle @@ -42,6 +42,8 @@ android { jvmTarget = '1.8' } namespace 'com.onesignal.inAppMessages' + + kotlinOptions.freeCompilerArgs += ['-module-name', namespace] } tasks.withType(Test) { diff --git a/OneSignalSDK/onesignal/location/build.gradle b/OneSignalSDK/onesignal/location/build.gradle index 8496a4a86..f66be7455 100644 --- a/OneSignalSDK/onesignal/location/build.gradle +++ b/OneSignalSDK/onesignal/location/build.gradle @@ -42,6 +42,8 @@ android { jvmTarget = '1.8' } namespace 'com.onesignal.location' + + kotlinOptions.freeCompilerArgs += ['-module-name', namespace] } tasks.withType(Test) { diff --git a/OneSignalSDK/onesignal/notifications/build.gradle b/OneSignalSDK/onesignal/notifications/build.gradle index 5cf80e97a..3dd557788 100644 --- a/OneSignalSDK/onesignal/notifications/build.gradle +++ b/OneSignalSDK/onesignal/notifications/build.gradle @@ -42,6 +42,8 @@ android { jvmTarget = '1.8' } namespace 'com.onesignal.notifications' + + kotlinOptions.freeCompilerArgs += ['-module-name', namespace] } tasks.withType(Test) { From 545e5cdab6f48c6cb02c75476371b6f366f4f081 Mon Sep 17 00:00:00 2001 From: Shepherd Date: Wed, 20 Sep 2023 15:30:21 -0400 Subject: [PATCH 4/8] Change notification time out log level to info --- .../internal/generation/impl/NotificationGenerationProcessor.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationProcessor.kt b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationProcessor.kt index 5bd0b59f8..f66883522 100644 --- a/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationProcessor.kt +++ b/OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/generation/impl/NotificationGenerationProcessor.kt @@ -116,7 +116,7 @@ internal class NotificationGenerationProcessor( }.join() } } catch (to: TimeoutCancellationException) { - Logging.error("notificationWillShowInForegroundHandler timed out, continuing with wantsToDisplay=$wantsToDisplay.", to) + Logging.info("notificationWillShowInForegroundHandler timed out, continuing with wantsToDisplay=$wantsToDisplay.", to) } catch (t: Throwable) { Logging.error("notificationWillShowInForegroundHandler threw an exception. Displaying normal OneSignal notification.", t) } From b65c0082a37815978c2c930f26511d62105a69d1 Mon Sep 17 00:00:00 2001 From: emawby Date: Thu, 21 Sep 2023 10:17:58 -0700 Subject: [PATCH 5/8] Fix crash from missing operationExecutor in OperationModelStore create I removed throwing from create for non-singleton model stores. I kept throw if a singleton model store is unable to create its model. --- .../com/onesignal/common/modeling/IModelStore.kt | 2 +- .../java/com/onesignal/common/modeling/ModelStore.kt | 2 +- .../onesignal/common/modeling/SingletonModelStore.kt | 2 +- .../internal/operations/impl/OperationModelStore.kt | 12 +++++++++--- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/IModelStore.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/IModelStore.kt index c3f38ecb8..28d742856 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/IModelStore.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/IModelStore.kt @@ -16,7 +16,7 @@ interface IModelStore : * * @param jsonObject The optional [JSONObject] to initialize the new model with. */ - fun create(jsonObject: JSONObject? = null): TModel + fun create(jsonObject: JSONObject? = null): TModel? /** * List the models that are owned by this model store. diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/ModelStore.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/ModelStore.kt index 91bb502c6..185b60415 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/ModelStore.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/ModelStore.kt @@ -126,7 +126,7 @@ abstract class ModelStore( val str = _prefs.getString(PreferenceStores.ONESIGNAL, PreferenceOneSignalKeys.MODEL_STORE_PREFIX + name, "[]") val jsonArray = JSONArray(str) for (index in 0 until jsonArray.length()) { - val newModel = create(jsonArray.getJSONObject(index)) + val newModel = create(jsonArray.getJSONObject(index)) ?: continue _models.add(newModel) // listen for changes to this model newModel.subscribe(this) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/SingletonModelStore.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/SingletonModelStore.kt index 4b1890110..73ba0aabd 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/SingletonModelStore.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/SingletonModelStore.kt @@ -24,7 +24,7 @@ open class SingletonModelStore( return model } - val createdModel = store.create() + val createdModel = store.create() ?: throw Exception("Unable to initialize model from store $store") createdModel.id = _singletonId store.add(createdModel) return createdModel diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationModelStore.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationModelStore.kt index 2a0319d07..4577ad7b0 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationModelStore.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationModelStore.kt @@ -3,11 +3,13 @@ package com.onesignal.core.internal.operations.impl import com.onesignal.common.modeling.ModelStore import com.onesignal.core.internal.operations.Operation import com.onesignal.core.internal.preferences.IPreferencesService +import com.onesignal.debug.internal.logging.Logging import com.onesignal.user.internal.operations.CreateSubscriptionOperation import com.onesignal.user.internal.operations.DeleteAliasOperation import com.onesignal.user.internal.operations.DeleteSubscriptionOperation import com.onesignal.user.internal.operations.DeleteTagOperation import com.onesignal.user.internal.operations.LoginUserOperation +import com.onesignal.user.internal.operations.LoginUserFromSubscriptionOperation import com.onesignal.user.internal.operations.RefreshUserOperation import com.onesignal.user.internal.operations.SetAliasOperation import com.onesignal.user.internal.operations.SetPropertyOperation @@ -18,6 +20,7 @@ import com.onesignal.user.internal.operations.TrackSessionStartOperation import com.onesignal.user.internal.operations.TransferSubscriptionOperation import com.onesignal.user.internal.operations.UpdateSubscriptionOperation import com.onesignal.user.internal.operations.impl.executors.IdentityOperationExecutor +import com.onesignal.user.internal.operations.impl.executors.LoginUserFromSubscriptionOperationExecutor import com.onesignal.user.internal.operations.impl.executors.LoginUserOperationExecutor import com.onesignal.user.internal.operations.impl.executors.RefreshUserOperationExecutor import com.onesignal.user.internal.operations.impl.executors.SubscriptionOperationExecutor @@ -30,13 +33,15 @@ internal class OperationModelStore(prefs: IPreferencesService) : ModelStore DeleteSubscriptionOperation() SubscriptionOperationExecutor.TRANSFER_SUBSCRIPTION -> TransferSubscriptionOperation() LoginUserOperationExecutor.LOGIN_USER -> LoginUserOperation() + LoginUserFromSubscriptionOperationExecutor.LOGIN_USER_FROM_SUBSCRIPTION_USER -> LoginUserFromSubscriptionOperation() RefreshUserOperationExecutor.REFRESH_USER -> RefreshUserOperation() UpdateUserOperationExecutor.SET_TAG -> SetTagOperation() UpdateUserOperationExecutor.DELETE_TAG -> DeleteTagOperation() From ea708832855e3c8d1da4a6bf669eb82a84b875bc Mon Sep 17 00:00:00 2001 From: Jenna Antilla <46546946+jennantilla@users.noreply.github.com> Date: Thu, 21 Sep 2023 12:10:09 -0700 Subject: [PATCH 6/8] Replace messageView non-null assertion with safe call operator Prevents NullPointerException crash when auto-dismiss IAM is manually closed. --- .../internal/display/impl/WebViewManager.kt | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/display/impl/WebViewManager.kt b/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/display/impl/WebViewManager.kt index b6cf4240f..442c3cb15 100644 --- a/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/display/impl/WebViewManager.kt +++ b/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/display/impl/WebViewManager.kt @@ -279,13 +279,14 @@ internal class WebViewManager( } Logging.debug("In app message, showing first one with height: $newHeight") - messageView!!.setWebView(webView!!) + messageView?.setWebView(webView!!) if (newHeight != null) { lastPageHeight = newHeight - messageView!!.updateHeight(newHeight) + messageView?.updateHeight(newHeight) } - messageView!!.showView(activity) - messageView!!.checkIfShouldDismiss() + messageView?.showView(activity) + // Executed in the same thread + messageView?.checkIfShouldDismiss() } } From cdc09c1f1b83c6369d4e85f25dd2c9bbadfc36b7 Mon Sep 17 00:00:00 2001 From: Jenna Antilla <46546946+jennantilla@users.noreply.github.com> Date: Fri, 22 Sep 2023 12:50:11 -0700 Subject: [PATCH 7/8] Clarify comment for showView execution --- .../inAppMessages/internal/display/impl/WebViewManager.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/display/impl/WebViewManager.kt b/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/display/impl/WebViewManager.kt index 442c3cb15..be750dbda 100644 --- a/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/display/impl/WebViewManager.kt +++ b/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/display/impl/WebViewManager.kt @@ -284,8 +284,8 @@ internal class WebViewManager( lastPageHeight = newHeight messageView?.updateHeight(newHeight) } + // showView does not return until in-app is dismissed messageView?.showView(activity) - // Executed in the same thread messageView?.checkIfShouldDismiss() } } From 7bd784f5aef0d6476147aa89db4ff526781b183a Mon Sep 17 00:00:00 2001 From: emawby Date: Fri, 22 Sep 2023 14:26:59 -0700 Subject: [PATCH 8/8] make getTimeZoneId non-optional --- .../core/src/main/java/com/onesignal/common/TimeUtils.kt | 2 +- .../operations/impl/executors/LoginUserOperationExecutor.kt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/TimeUtils.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/TimeUtils.kt index d126ecc21..a71fcdb68 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/TimeUtils.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/TimeUtils.kt @@ -17,7 +17,7 @@ object TimeUtils { return offset / 1000 } - fun getTimeZoneId(): String? { + fun getTimeZoneId(): String { return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { ZoneId.systemDefault().id } else { diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/LoginUserOperationExecutor.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/LoginUserOperationExecutor.kt index a3535ddf3..37c7d40f7 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/LoginUserOperationExecutor.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/LoginUserOperationExecutor.kt @@ -94,7 +94,7 @@ internal class LoginUserOperationExecutor( var identities = mapOf() var subscriptions = mapOf() val properties = mutableMapOf() - properties["timezone_id"] = TimeUtils.getTimeZoneId()!! + properties["timezone_id"] = TimeUtils.getTimeZoneId() if (createUserOperation.externalId != null) { val mutableIdentities = identities.toMutableMap()