Skip to content
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.

Sign up for GitHub

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

[Feat] Use only OneSignal ID for requests #1464

Merged
merged 10 commits into from
Aug 2, 2024
Merged

[Feat] Use only OneSignal ID for requests #1464

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
658663c
Deprecate Transfer Subscription
nan-li Jul 24, 2024
e656417
Use Create User to hydrate OneSignal ID
nan-li Jul 24, 2024
454909d
Fetch User by OSID, not external ID
nan-li Jul 24, 2024
20e9bb9
Change handling of Identify User 409 conflict
nan-li Jul 24, 2024
051f29b
Identify User success will hydrate OSID + EUID
nan-li Jul 24, 2024
5f43d4f
All requests will only use OneSignal ID
nan-li Jul 23, 2024
4ed7a64
Remove concept of primary alias
nan-li Jul 24, 2024
589bc34
[tests] Update tests
nan-li Jul 23, 2024
0b6ddc8
Refactor create user to make the alias explicit
nan-li Jul 25, 2024
bc9112c
Fix a bug to fetch user after create user is made
nan-li Jul 25, 2024
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
19 changes: 17 additions & 2 deletions iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,20 @@ extension OSUserExecutor {
executePendingRequests()
}

/**
This Create User call expects an Identity Model with external ID to hydrate the OneSignal ID
*/
static func createUser(identityModel: OSIdentityModel) {
guard identityModel.externalId != nil else {
OneSignalLog.onesignalLog(.LL_ERROR, message: "createUser(identityModel) called with missing external ID")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we hit this case what happens? Will we try again in the same session?

Copy link
Contributor Author

@nan-li nan-li Jul 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose of this version of ceateUser() is to get an OSID for an EUID. The EUID must exist because the only usage of this method is after an Identify User 409 conflict, in which case we would have the EUID. The guard and error log is more for noticing that we do something wrong such as using this method inappropriately down the line.

Thinking about this again, it may be clearer to make EUID a parameter instead of it being implicit.

No, it won't ever retry, which means any pending updates for a previous user who experienced this particular flow will get updates dropped.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ya it should probably require it as a param. Would it retry next session?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I edited my response after to talk about retrying. This call would be made if the user has changed since, so this would be for a previous user. It won't retry since no EUID, no way to get its OSID. That means if there are pending requests for this previous user, they will effectively be dropped.

return
}

let request = OSRequestCreateUser(identityModel: identityModel, propertiesModel: nil, pushSubscriptionModel: nil, originalPushToken: nil)
appendToQueue(request)
executePendingRequests()
}

static func executeCreateUserRequest(_ request: OSRequestCreateUser) {
guard !request.sentToClient else {
return
Expand All @@ -200,8 +214,9 @@ extension OSUserExecutor {
}
request.sentToClient = true

// Hook up push subscription model, it may be updated with a subscription_id, etc.
if let pushSubscriptionModel = OneSignalUserManagerImpl.sharedInstance.pushSubscriptionModelStore.getModel(modelId: request.pushSubscriptionModel.modelId) {
// Hook up push subscription model if exists, it may be updated with a subscription_id, etc.
if let modelId = request.pushSubscriptionModel?.modelId,
let pushSubscriptionModel = OneSignalUserManagerImpl.sharedInstance.pushSubscriptionModelStore.getModel(modelId: modelId) {
request.pushSubscriptionModel = pushSubscriptionModel
request.updatePushSubscriptionModel(pushSubscriptionModel)
}
Expand Down
29 changes: 18 additions & 11 deletions iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestCreateUser.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ import OneSignalCore
/**
This request will be made with the minimum information needed. The payload will contain an externalId or no identities.
The push subscription may or may not have a token or suscriptionId already.
There will be no properties sent.
This request is used for typical User Create, which will include properties and the push subscription,
or to hydrate OneSignal ID for a given External ID, which will only contain the Identity object in the payload.
*/
class OSRequestCreateUser: OneSignalRequest, OSUserRequest {
var sentToClient = false
Expand All @@ -40,7 +41,7 @@ class OSRequestCreateUser: OneSignalRequest, OSUserRequest {
}

var identityModel: OSIdentityModel
var pushSubscriptionModel: OSSubscriptionModel
var pushSubscriptionModel: OSSubscriptionModel?
var originalPushToken: String?

func prepareForExecution() -> Bool {
Expand All @@ -55,14 +56,17 @@ class OSRequestCreateUser: OneSignalRequest, OSUserRequest {
return true
}

// When reading from the cache, update the push subscription model
// When reading from the cache, update the push subscription model, if appropriate
func updatePushSubscriptionModel(_ pushSubscriptionModel: OSSubscriptionModel) {
guard self.pushSubscriptionModel != nil else {
return
}
self.pushSubscriptionModel = pushSubscriptionModel
self.parameters?["subscriptions"] = [pushSubscriptionModel.jsonRepresentation()]
self.originalPushToken = pushSubscriptionModel.address
}

init(identityModel: OSIdentityModel, propertiesModel: OSPropertiesModel, pushSubscriptionModel: OSSubscriptionModel, originalPushToken: String?) {
init(identityModel: OSIdentityModel, propertiesModel: OSPropertiesModel?, pushSubscriptionModel: OSSubscriptionModel?, originalPushToken: String?) {
self.identityModel = identityModel
self.pushSubscriptionModel = pushSubscriptionModel
self.originalPushToken = originalPushToken
Expand All @@ -78,14 +82,18 @@ class OSRequestCreateUser: OneSignalRequest, OSUserRequest {
}

// Properties Object
var propertiesObject: [String: Any] = [:]
propertiesObject["language"] = propertiesModel.language
propertiesObject["timezone_id"] = propertiesModel.timezoneId
params["properties"] = propertiesObject
if let propertiesModel = propertiesModel {
var propertiesObject: [String: Any] = [:]
propertiesObject["language"] = propertiesModel.language
propertiesObject["timezone_id"] = propertiesModel.timezoneId
params["properties"] = propertiesObject
}

params["refresh_device_metadata"] = true
self.parameters = params
self.updatePushSubscriptionModel(pushSubscriptionModel)
if let pushSub = pushSubscriptionModel {
self.updatePushSubscriptionModel(pushSub)
}
self.method = POST
}

Expand All @@ -102,7 +110,6 @@ class OSRequestCreateUser: OneSignalRequest, OSUserRequest {
required init?(coder: NSCoder) {
guard
let identityModel = coder.decodeObject(forKey: "identityModel") as? OSIdentityModel,
let pushSubscriptionModel = coder.decodeObject(forKey: "pushSubscriptionModel") as? OSSubscriptionModel,
let parameters = coder.decodeObject(forKey: "parameters") as? [String: Any],
let rawMethod = coder.decodeObject(forKey: "method") as? UInt32,
let path = coder.decodeObject(forKey: "path") as? String,
Expand All @@ -112,7 +119,7 @@ class OSRequestCreateUser: OneSignalRequest, OSUserRequest {
return nil
}
self.identityModel = identityModel
self.pushSubscriptionModel = pushSubscriptionModel
self.pushSubscriptionModel = coder.decodeObject(forKey: "pushSubscriptionModel") as? OSSubscriptionModel
self.originalPushToken = coder.decodeObject(forKey: "originalPushToken") as? String
self.stringDescription = "<OSRequestCreateUser with externalId: \(identityModel.externalId ?? "nil")>"
super.init()
Expand Down