Skip to content

Commit 9911f8e

Browse files
brismithersbrismithers
committed
code review comments and unit testing
1 parent c67b8e1 commit 9911f8e

File tree

13 files changed

+89
-65
lines changed

13 files changed

+89
-65
lines changed

‎OneSignalSDK/onesignal/inAppMessages/src/main/java/com/onesignal/inAppMessages/internal/InAppMessagesManager.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,9 +167,9 @@ internal class InAppMessagesManager(
167167
}
168168
}
169169

170-
override fun onSubscriptionsAdded(subscription: ISubscription) { }
170+
override fun onSubscriptionAdded(subscription: ISubscription) { }
171171
override fun onSubscriptionRemoved(subscription: ISubscription) { }
172-
override fun onSubscriptionsChanged(subscription: ISubscription, args: ModelChangedArgs) {
172+
override fun onSubscriptionChanged(subscription: ISubscription, args: ModelChangedArgs) {
173173
if (subscription !is IPushSubscription || args.path != SubscriptionModel::id.name) {
174174
return
175175
}

‎OneSignalSDK/onesignal/inAppMessages/src/test/java/com/onesignal/inAppMessages/internal/backend/InAppBackendServiceTests.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ class InAppBackendServiceTests : FunSpec({
5151
/* Given */
5252
val mockHydrator = InAppHydrator(MockHelper.time(1000), MockHelper.propertiesModelStore())
5353
val mockHttpClient = mockk<IHttpClient>()
54-
coEvery { mockHttpClient.get(any(), any()) } returns HttpResponse(200, "{ in_app_messages: [{id: \"messageId1\", variants:{all: {en: \"content1\"}}, triggers:[[{id: \"triggerId1\", kind: \"custom\", property: \"property1\", operator: \"equal\", value: \"value1\"}]], end_time: \"2020-12-13T23:23:23\", redisplay: { limit: 11111, delay: 22222}] }")
54+
coEvery { mockHttpClient.get(any(), any()) } returns HttpResponse(200, "{ in_app_messages: [{id: \"messageId1\", variants:{all: {en: \"content1\"}}, triggers:[[{id: \"triggerId1\", kind: \"custom\", property: \"property1\", operator: \"equal\", value: \"value1\"}]], end_time: \"2008-09-03T20:56:35.450686Z\", redisplay: { limit: 11111, delay: 22222}}] }")
5555

5656
val inAppBackendService = InAppBackendService(mockHttpClient, MockHelper.deviceService(), mockHydrator)
5757

@@ -62,9 +62,9 @@ class InAppBackendServiceTests : FunSpec({
6262
response shouldNotBe null
6363
response!!.count() shouldBe 1
6464
response[0].messageId shouldBe "messageId1"
65-
response[0].variants.keys shouldBe 1
65+
response[0].variants.keys.count() shouldBe 1
6666
response[0].variants["all"] shouldNotBe null
67-
response[0].variants["all"]!!.keys shouldBe 1
67+
response[0].variants["all"]!!.keys.count() shouldBe 1
6868
response[0].variants["all"]!!["en"] shouldBe "content1"
6969
response[0].triggers.count() shouldBe 1
7070
response[0].triggers[0].count() shouldBe 1

‎OneSignalSDK/onesignal/notifications/src/main/java/com/onesignal/notifications/internal/listeners/DeviceRegistrationListener.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,8 @@ internal class DeviceRegistrationListener(
8080
}
8181

8282
override fun onSubscriptionRemoved(subscription: ISubscription) { }
83-
override fun onSubscriptionsAdded(subscription: ISubscription) { }
84-
override fun onSubscriptionsChanged(subscription: ISubscription, args: ModelChangedArgs) {
83+
override fun onSubscriptionAdded(subscription: ISubscription) { }
84+
override fun onSubscriptionChanged(subscription: ISubscription, args: ModelChangedArgs) {
8585
// when going from optedIn=false to optedIn=true and there aren't permissions, automatically drive
8686
// permission request.
8787
if (args.path == SubscriptionModel::optedIn.name && args.oldValue == false && args.newValue == true && !_notificationsManager.permission) {

‎OneSignalSDK/onesignal/src/main/java/com/onesignal/user/internal/backend/IIdentityBackendService.kt

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
package com.onesignal.user.internal.backend
22

3+
import com.onesignal.common.exceptions.BackendException
4+
35
interface IIdentityBackendService {
46
/**
5-
* Create one or more aliases for the user identified by the [aliasLabel]/[aliasValue] provided. For
6-
* each being created, the [aliasLabel]/[aliasValue] pair must not already exist within the [appId].
7+
* Set one or more aliases for the user identified by the [aliasLabel]/[aliasValue] provided.
78
*
89
* If there is a non-successful response from the backend, a [BackendException] will be thrown with response data.
910
*
@@ -12,7 +13,7 @@ interface IIdentityBackendService {
1213
* @param aliasValue The identifier within the [aliasLabel] that identifies the user to retrieve.
1314
* @param identities The identities that are to be created.
1415
*/
15-
suspend fun createAlias(appId: String, aliasLabel: String, aliasValue: String, identities: Map<String, String>): Map<String, String>
16+
suspend fun setAlias(appId: String, aliasLabel: String, aliasValue: String, identities: Map<String, String>): Map<String, String>
1617

1718
/**
1819
* Delete the [aliasLabelToDelete] from the user identified by the [aliasLabel]/[aliasValue] provided.

‎OneSignalSDK/onesignal/src/main/java/com/onesignal/user/internal/backend/impl/IdentityBackendService.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import org.json.JSONObject
1010
internal class IdentityBackendService(
1111
private val _httpClient: IHttpClient
1212
) : IIdentityBackendService {
13-
override suspend fun createAlias(appId: String, aliasLabel: String, aliasValue: String, identities: Map<String, String>): Map<String, String> {
13+
override suspend fun setAlias(appId: String, aliasLabel: String, aliasValue: String, identities: Map<String, String>): Map<String, String> {
1414
val requestJSONObject = JSONObject()
1515
.put("identity", JSONObject().putMap(identities))
1616

‎OneSignalSDK/onesignal/src/main/java/com/onesignal/user/internal/operations/impl/executors/IdentityOperationExecutor.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ internal class IdentityOperationExecutor(
3232

3333
if (lastOperation is SetAliasOperation) {
3434
try {
35-
_identityBackend.createAlias(
35+
_identityBackend.setAlias(
3636
lastOperation.appId,
3737
IdentityConstants.ONESIGNAL_ID,
3838
lastOperation.onesignalId,

‎OneSignalSDK/onesignal/src/main/java/com/onesignal/user/internal/subscriptions/ISubscriptionManager.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ interface ISubscriptionManager : IEventNotifier<ISubscriptionChangedHandler> {
1717
}
1818

1919
interface ISubscriptionChangedHandler {
20-
fun onSubscriptionsAdded(subscription: ISubscription)
21-
fun onSubscriptionsChanged(subscription: ISubscription, args: ModelChangedArgs)
20+
fun onSubscriptionAdded(subscription: ISubscription)
21+
fun onSubscriptionChanged(subscription: ISubscription, args: ModelChangedArgs)
2222
fun onSubscriptionRemoved(subscription: ISubscription)
2323
}

‎OneSignalSDK/onesignal/src/main/java/com/onesignal/user/internal/subscriptions/impl/SubscriptionManager.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ internal class SubscriptionManager(
141141
}
142142

143143
// the model has already been updated, so fire the update event
144-
_events.fire { it.onSubscriptionsChanged(subscription, args) }
144+
_events.fire { it.onSubscriptionChanged(subscription, args) }
145145
}
146146
}
147147

@@ -172,7 +172,7 @@ internal class SubscriptionManager(
172172
subscriptions.add(subscription)
173173
this.subscriptions = SubscriptionList(subscriptions, UninitializedPushSubscription())
174174

175-
_events.fire { it.onSubscriptionsAdded(subscription) }
175+
_events.fire { it.onSubscriptionAdded(subscription) }
176176
}
177177

178178
private fun removeSubscriptionFromSubscriptionList(subscription: ISubscription) {

‎OneSignalSDK/onesignal/src/test/java/com/onesignal/user/internal/backend/IdentityBackendServiceTests.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ class IdentityBackendServiceTests : FunSpec({
2929
val identities = mapOf("aliasKey1" to "aliasValue1")
3030

3131
/* When */
32-
val response = identityBackendService.createAlias("appId", aliasLabel, aliasValue, identities)
32+
val response = identityBackendService.setAlias("appId", aliasLabel, aliasValue, identities)
3333

3434
/* Then */
3535
response["aliasKey1"] shouldBe "aliasValue1"

‎OneSignalSDK/onesignal/src/test/java/com/onesignal/user/internal/operations/IdentityOperationExecutorTests.kt

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,11 @@ class IdentityOperationExecutorTests : FunSpec({
2828
test("execution of set alias operation") {
2929
/* Given */
3030
val mockIdentityBackendService = mockk<IIdentityBackendService>()
31-
coEvery { mockIdentityBackendService.createAlias(any(), any(), any(), any()) } returns mapOf()
31+
coEvery { mockIdentityBackendService.setAlias(any(), any(), any(), any()) } returns mapOf()
3232

3333
val mockIdentityModel = mockk<IdentityModel>()
3434
every { mockIdentityModel.onesignalId } returns "onesignalId"
35-
every { mockIdentityModel.setProperty<String>(any(), any(), any()) } just runs
35+
every { mockIdentityModel.setStringProperty(any(), any(), any()) } just runs
3636

3737
val mockIdentityModelStore = mockk<IdentityModelStore>()
3838
every { mockIdentityModelStore.model } returns mockIdentityModel
@@ -45,14 +45,14 @@ class IdentityOperationExecutorTests : FunSpec({
4545

4646
/* Then */
4747
response.result shouldBe ExecutionResult.SUCCESS
48-
coVerify(exactly = 1) { mockIdentityBackendService.createAlias("appId", IdentityConstants.ONESIGNAL_ID, "onesignalId", mapOf("aliasKey1" to "aliasValue1")) }
49-
verify(exactly = 1) { mockIdentityModel.setProperty("aliasKey1", "aliasValue1", ModelChangeTags.HYDRATE) }
48+
coVerify(exactly = 1) { mockIdentityBackendService.setAlias("appId", IdentityConstants.ONESIGNAL_ID, "onesignalId", mapOf("aliasKey1" to "aliasValue1")) }
49+
verify(exactly = 1) { mockIdentityModel.setStringProperty("aliasKey1", "aliasValue1", ModelChangeTags.HYDRATE) }
5050
}
5151

5252
test("execution of set alias operation with network timeout") {
5353
/* Given */
5454
val mockIdentityBackendService = mockk<IIdentityBackendService>()
55-
coEvery { mockIdentityBackendService.createAlias(any(), any(), any(), any()) } throws BackendException(408, "TIMEOUT")
55+
coEvery { mockIdentityBackendService.setAlias(any(), any(), any(), any()) } throws BackendException(408, "TIMEOUT")
5656

5757
val mockIdentityModelStore = MockHelper.identityModelStore()
5858

@@ -70,7 +70,7 @@ class IdentityOperationExecutorTests : FunSpec({
7070
test("execution of set alias operation with non-retryable error") {
7171
/* Given */
7272
val mockIdentityBackendService = mockk<IIdentityBackendService>()
73-
coEvery { mockIdentityBackendService.createAlias(any(), any(), any(), any()) } throws BackendException(404, "NOT FOUND")
73+
coEvery { mockIdentityBackendService.setAlias(any(), any(), any(), any()) } throws BackendException(404, "NOT FOUND")
7474

7575
val mockIdentityModelStore = MockHelper.identityModelStore()
7676

@@ -92,7 +92,7 @@ class IdentityOperationExecutorTests : FunSpec({
9292

9393
val mockIdentityModel = mockk<IdentityModel>()
9494
every { mockIdentityModel.onesignalId } returns "onesignalId"
95-
every { mockIdentityModel.setProperty<String>(any(), any(), any()) } just runs
95+
every { mockIdentityModel.setOptStringProperty(any(), any(), any()) } just runs
9696

9797
val mockIdentityModelStore = mockk<IdentityModelStore>()
9898
every { mockIdentityModelStore.model } returns mockIdentityModel
@@ -106,7 +106,7 @@ class IdentityOperationExecutorTests : FunSpec({
106106
/* Then */
107107
response.result shouldBe ExecutionResult.SUCCESS
108108
coVerify(exactly = 1) { mockIdentityBackendService.deleteAlias("appId", IdentityConstants.ONESIGNAL_ID, "onesignalId", "aliasKey1") }
109-
verify(exactly = 1) { mockIdentityModel.setProperty("aliasKey1", null, ModelChangeTags.HYDRATE) }
109+
verify(exactly = 1) { mockIdentityModel.setOptStringProperty("aliasKey1", null, ModelChangeTags.HYDRATE) }
110110
}
111111

112112
test("execution of delete alias operation with network timeout") {

0 commit comments

Comments
 (0)