Skip to content

Delete Subscription by Type #2322

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 6 commits into
base: identity_verification_beta
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package com.onesignal.user.internal.backend

import com.onesignal.common.consistency.RywData
import com.onesignal.common.exceptions.BackendException
import com.onesignal.user.internal.subscriptions.SubscriptionType

interface ISubscriptionBackendService {
/**
Expand Down Expand Up @@ -42,11 +43,13 @@ interface ISubscriptionBackendService {
* Delete an existing subscription.
*
* @param appId The ID of the OneSignal application this subscription exists under.
* @param subscriptionId The ID of the subscription to delete.
* @param subscriptionType The type of the subscription to delete.
* @param subscriptionToken The token/address of the subscription to delete.
*/
suspend fun deleteSubscription(
appId: String,
subscriptionId: String,
subscriptionType: SubscriptionType,
subscriptionToken: String,
jwt: String? = null,
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import com.onesignal.core.internal.http.IHttpClient
import com.onesignal.core.internal.http.impl.OptionalHeaders
import com.onesignal.user.internal.backend.ISubscriptionBackendService
import com.onesignal.user.internal.backend.SubscriptionObject
import com.onesignal.user.internal.subscriptions.SubscriptionType
import org.json.JSONObject
import java.net.URLEncoder

internal class SubscriptionBackendService(
private val _httpClient: IHttpClient,
Expand Down Expand Up @@ -87,10 +89,13 @@ internal class SubscriptionBackendService(

override suspend fun deleteSubscription(
appId: String,
subscriptionId: String,
subscriptionType: SubscriptionType,
subscriptionToken: String,
jwt: String?,
) {
val response = _httpClient.delete("apps/$appId/subscriptions/$subscriptionId", OptionalHeaders(jwt = jwt))
val type = subscriptionType.name.lowercase()
val encodedToken = URLEncoder.encode(subscriptionToken, "UTF-8")
val response = _httpClient.delete("apps/$appId/by/type/$type/token/$encodedToken/subscriptions", OptionalHeaders(jwt = jwt))

if (!response.isSuccess) {
throw BackendException(response.statusCode, response.payload, response.retryAfterSeconds)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import com.onesignal.common.IDManager
import com.onesignal.core.internal.operations.GroupComparisonType
import com.onesignal.core.internal.operations.Operation
import com.onesignal.user.internal.operations.impl.executors.SubscriptionOperationExecutor
import com.onesignal.user.internal.subscriptions.SubscriptionType

/**
* An [Operation] to delete a subscription from the OneSignal backend.
Expand Down Expand Up @@ -38,16 +39,51 @@ class DeleteSubscriptionOperation() : Operation(SubscriptionOperationExecutor.DE
setStringProperty(::subscriptionId.name, value)
}

/**
* The type of subscription.
*/
var type: SubscriptionType
get() = getEnumProperty(::type.name)
private set(value) {
setEnumProperty(::type.name, value)
}

/**
* The address-specific information for this subscription. Its contents depends on the type
* of subscription:
*
* * [SubscriptionType.EMAIL]: An email address.
* * [SubscriptionType.SMS]: A phone number in E.164 format.
*/
var address: String
get() = getStringProperty(::address.name)
private set(value) {
setStringProperty(::address.name, value)
}

override val createComparisonKey: String get() = "$appId.User.$onesignalId"
override val modifyComparisonKey: String get() = "$appId.User.$onesignalId.Subscription.$subscriptionId"
override val groupComparisonType: GroupComparisonType = GroupComparisonType.NONE
override val canStartExecute: Boolean get() = !IDManager.isLocalId(onesignalId) && !IDManager.isLocalId(subscriptionId)
override val canStartExecute: Boolean
get() =
!IDManager.isLocalId(onesignalId) &&
!IDManager.isLocalId(
subscriptionId,
)
override val applyToRecordId: String get() = subscriptionId

constructor(appId: String, onesignalId: String, subscriptionId: String) : this() {
constructor(
appId: String,
onesignalId: String,
subscriptionId: String,
type: SubscriptionType,
address: String,
) : this() {
this.appId = appId
this.onesignalId = onesignalId
this.subscriptionId = subscriptionId
this.type = type
this.address = address
}

override fun translateIds(map: Map<String, String>) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ internal class SubscriptionOperationExecutor(

private suspend fun deleteSubscription(op: DeleteSubscriptionOperation): ExecutionResponse {
try {
_subscriptionBackend.deleteSubscription(op.appId, op.subscriptionId, _identityModelStore.model.jwtToken)
_subscriptionBackend.deleteSubscription(op.appId, op.type, op.address, _identityModelStore.model.jwtToken)

// remove the subscription model as a HYDRATE in case for some reason it still exists.
_subscriptionModelStore.remove(op.subscriptionId, ModelChangeTags.HYDRATE)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ internal class SubscriptionModelStoreListener(
}

override fun getRemoveOperation(model: SubscriptionModel): Operation {
return DeleteSubscriptionOperation(_configModelStore.model.appId, _identityModelStore.model.onesignalId, model.id)
return DeleteSubscriptionOperation(_configModelStore.model.appId, _identityModelStore.model.onesignalId, model.id, model.type, model.address)
}

override fun getUpdateOperation(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import com.onesignal.debug.LogLevel
import com.onesignal.debug.internal.logging.Logging
import com.onesignal.user.internal.backend.impl.SubscriptionBackendService
import com.onesignal.user.internal.subscriptions.SubscriptionStatus
import com.onesignal.user.internal.subscriptions.SubscriptionType
import io.kotest.assertions.throwables.shouldThrowUnit
import io.kotest.core.spec.style.FunSpec
import io.kotest.matchers.shouldBe
Expand Down Expand Up @@ -107,4 +108,45 @@ class SubscriptionBackendServiceTests : FunSpec({
)
}
}

test("delete subscription by type and token successfully") {
// Given
val spyHttpClient = mockk<IHttpClient>()
coEvery { spyHttpClient.delete(any(), any()) } returns HttpResponse(200, "")
val subscriptionBackendService = SubscriptionBackendService(spyHttpClient)

// When
subscriptionBackendService.deleteSubscription("appId", SubscriptionType.EMAIL, "[email protected]", "jwt-token")

// Then
coVerify {
spyHttpClient.delete(
"apps/appId/by/type/email/token/test%40example.com/subscriptions",
any(),
)
}
}

test("delete subscription throws exception when bad response") {
// Given
val spyHttpClient = mockk<IHttpClient>()
coEvery { spyHttpClient.delete(any(), any()) } returns HttpResponse(404, "NOT FOUND")
val subscriptionBackendService = SubscriptionBackendService(spyHttpClient)

// When
val exception =
shouldThrowUnit<BackendException> {
subscriptionBackendService.deleteSubscription("appId", SubscriptionType.SMS, "+1234567890")
}

// Then
exception.statusCode shouldBe 404
exception.response shouldBe "NOT FOUND"
coVerify {
spyHttpClient.delete(
"apps/appId/by/type/sms/token/%2B1234567890/subscriptions",
any(),
)
}
}
})
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ class LoginUserOperationExecutorTests : FunSpec({
"[email protected]",
SubscriptionStatus.SUBSCRIBED,
),
DeleteSubscriptionOperation(appId, localOneSignalId, "subscriptionId2"),
DeleteSubscriptionOperation(appId, localOneSignalId, "subscriptionId2", SubscriptionType.EMAIL, "[email protected]"),
)

// When
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ class SubscriptionOperationExecutorTests :
"pushToken",
SubscriptionStatus.SUBSCRIBED,
),
DeleteSubscriptionOperation(appId, remoteOneSignalId, localSubscriptionId),
DeleteSubscriptionOperation(appId, remoteOneSignalId, localSubscriptionId, SubscriptionType.PUSH, "pushToken"),
)

// When
Expand Down Expand Up @@ -606,7 +606,7 @@ class SubscriptionOperationExecutorTests :
test("delete subscription successfully deletes subscription") {
// Given
val mockSubscriptionBackendService = mockk<ISubscriptionBackendService>()
coEvery { mockSubscriptionBackendService.deleteSubscription(any(), any()) } just runs
coEvery { mockSubscriptionBackendService.deleteSubscription(any<String>(), any<SubscriptionType>(), any<String>(), any()) } just runs

val mockIdentityModelStore = MockHelper.identityModelStore()
val mockSubscriptionsModelStore = mockk<SubscriptionModelStore>()
Expand All @@ -629,22 +629,22 @@ class SubscriptionOperationExecutorTests :

val operations =
listOf<Operation>(
DeleteSubscriptionOperation(appId, remoteOneSignalId, remoteSubscriptionId),
DeleteSubscriptionOperation(appId, remoteOneSignalId, remoteSubscriptionId, SubscriptionType.PUSH, "pushToken"),
)

// When
val response = subscriptionOperationExecutor.execute(operations)

// Then
response.result shouldBe ExecutionResult.SUCCESS
coVerify(exactly = 1) { mockSubscriptionBackendService.deleteSubscription(appId, remoteSubscriptionId) }
coVerify(exactly = 1) { mockSubscriptionBackendService.deleteSubscription(appId, SubscriptionType.PUSH, "pushToken", any()) }
verify(exactly = 1) { mockSubscriptionsModelStore.remove(remoteSubscriptionId, any()) }
}

test("delete subscription fails with retry when there is a network condition") {
// Given
val mockSubscriptionBackendService = mockk<ISubscriptionBackendService>()
coEvery { mockSubscriptionBackendService.deleteSubscription(any(), any()) } throws BackendException(408)
coEvery { mockSubscriptionBackendService.deleteSubscription(any<String>(), any<SubscriptionType>(), any<String>(), any()) } throws BackendException(408)

val mockIdentityModelStore = MockHelper.identityModelStore()
val mockSubscriptionsModelStore = mockk<SubscriptionModelStore>()
Expand All @@ -665,23 +665,23 @@ class SubscriptionOperationExecutorTests :

val operations =
listOf<Operation>(
DeleteSubscriptionOperation(appId, remoteOneSignalId, remoteSubscriptionId),
DeleteSubscriptionOperation(appId, remoteOneSignalId, remoteSubscriptionId, SubscriptionType.PUSH, "pushToken"),
)

// When
val response = subscriptionOperationExecutor.execute(operations)

// Then
response.result shouldBe ExecutionResult.FAIL_RETRY
coVerify(exactly = 1) { mockSubscriptionBackendService.deleteSubscription(appId, remoteSubscriptionId) }
coVerify(exactly = 1) { mockSubscriptionBackendService.deleteSubscription(appId, SubscriptionType.PUSH, "pushToken", any()) }
}

// If we get a 404 then the subscription has already been deleted,
// so we count it as successful
test("delete subscription is successful if there is a 404") {
// Given
val mockSubscriptionBackendService = mockk<ISubscriptionBackendService>()
coEvery { mockSubscriptionBackendService.deleteSubscription(any(), any()) } throws BackendException(404)
coEvery { mockSubscriptionBackendService.deleteSubscription(any<String>(), any<SubscriptionType>(), any<String>(), any()) } throws BackendException(404)

val mockIdentityModelStore = MockHelper.identityModelStore()
val mockSubscriptionsModelStore = mockk<SubscriptionModelStore>()
Expand All @@ -702,21 +702,21 @@ class SubscriptionOperationExecutorTests :

val operations =
listOf<Operation>(
DeleteSubscriptionOperation(appId, remoteOneSignalId, remoteSubscriptionId),
DeleteSubscriptionOperation(appId, remoteOneSignalId, remoteSubscriptionId, SubscriptionType.PUSH, "pushToken"),
)

// When
val response = subscriptionOperationExecutor.execute(operations)

// Then
response.result shouldBe ExecutionResult.SUCCESS
coVerify(exactly = 1) { mockSubscriptionBackendService.deleteSubscription(appId, remoteSubscriptionId) }
coVerify(exactly = 1) { mockSubscriptionBackendService.deleteSubscription(appId, SubscriptionType.PUSH, "pushToken", any()) }
}

test("delete subscription fails with retry when the backend returns MISSING, when isInMissingRetryWindow") {
// Given
val mockSubscriptionBackendService = mockk<ISubscriptionBackendService>()
coEvery { mockSubscriptionBackendService.deleteSubscription(any(), any()) } throws BackendException(404)
coEvery { mockSubscriptionBackendService.deleteSubscription(any<String>(), any<SubscriptionType>(), any<String>(), any()) } throws BackendException(404)

val mockSubscriptionsModelStore = mockk<SubscriptionModelStore>()
val mockBuildUserService = mockk<IRebuildUserService>()
Expand All @@ -738,7 +738,7 @@ class SubscriptionOperationExecutorTests :

val operations =
listOf<Operation>(
DeleteSubscriptionOperation(appId, remoteOneSignalId, remoteSubscriptionId),
DeleteSubscriptionOperation(appId, remoteOneSignalId, remoteSubscriptionId, SubscriptionType.PUSH, "pushToken"),
)

// When
Expand Down
Loading