Skip to content

persistent deviceId #910

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
1 change: 1 addition & 0 deletions swift-sdk/Core/Constants.swift
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ enum Const {
static let email = "itbl_email"
static let userId = "itbl_userid"
static let authToken = "itbl_auth_token"
static let deviceId = "itbl_device_id"
}
}

Expand Down
8 changes: 1 addition & 7 deletions swift-sdk/Internal/InternalIterableAPI.swift
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,7 @@ final class InternalIterableAPI: NSObject, PushTrackerProtocol, AuthProvider {
}

var deviceId: String {
if let value = localStorage.deviceId {
return value
} else {
let value = IterableUtil.generateUUID()
localStorage.deviceId = value
return value
}
localStorage.deviceId
}

var deviceMetadata: DeviceMetadata {
Expand Down
18 changes: 18 additions & 0 deletions swift-sdk/Internal/Utilities/Keychain/IterableKeychain.swift
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,24 @@ class IterableKeychain {
}
}

var deviceId: String? {
get {
let data = wrapper.data(forKey: Const.Keychain.Key.deviceId)

return data.flatMap { String(data: $0, encoding: .utf8) }
}

set {
guard let deviceId = newValue,
let data = deviceId.data(using: .utf8) else {
wrapper.removeValue(forKey: Const.Keychain.Key.deviceId)
return
}

wrapper.set(data, forKey: Const.Keychain.Key.deviceId)
}
}

// MARK: - PRIVATE/INTERNAL

private let wrapper: KeychainWrapper
Expand Down
20 changes: 16 additions & 4 deletions swift-sdk/Internal/Utilities/LocalStorage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
//

import Foundation
import UIKit.UIDevice

struct LocalStorage: LocalStorageProtocol {
init(userDefaults: UserDefaults = UserDefaults.standard,
Expand Down Expand Up @@ -43,11 +44,22 @@ struct LocalStorage: LocalStorageProtocol {
}
}

var deviceId: String? {
var deviceId: String {
get {
iterableUserDefaults.deviceId
} set {
iterableUserDefaults.deviceId = newValue
let foundDeviceId = [
iterableUserDefaults.deviceId,
keychain.deviceId,
UIDevice.current.identifierForVendor?.uuidString
].compactMap { $0 }.first
let deviceId = foundDeviceId ?? IterableUtil.generateUUID()

if iterableUserDefaults.deviceId == nil {
iterableUserDefaults.deviceId = deviceId
}
if keychain.deviceId == nil {
keychain.deviceId = deviceId
}
return deviceId
Comment on lines +49 to +62
Copy link
Preview

Copilot AI Apr 2, 2025

Choose a reason for hiding this comment

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

The getter for 'deviceId' modifies state by writing to user defaults and keychain, which may lead to unexpected side effects. Consider refactoring this logic so that state mutations occur in an explicit initialization or setter method rather than within a property getter.

Suggested change
let foundDeviceId = [
iterableUserDefaults.deviceId,
keychain.deviceId,
UIDevice.current.identifierForVendor?.uuidString
].compactMap { $0 }.first
let deviceId = foundDeviceId ?? IterableUtil.generateUUID()
if iterableUserDefaults.deviceId == nil {
iterableUserDefaults.deviceId = deviceId
}
if keychain.deviceId == nil {
keychain.deviceId = deviceId
}
return deviceId
return iterableUserDefaults.deviceId ?? keychain.deviceId ?? UIDevice.current.identifierForVendor?.uuidString ?? ""

Copilot uses AI. Check for mistakes.

Copy link
Author

Choose a reason for hiding this comment

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

No, provided code's logic is correct, we want to filter out first non nil from array of values, but if it fails generateUUID will be called, also you deleted persistency to UserDefaults and/or Keychain

}
}

Expand Down
2 changes: 1 addition & 1 deletion swift-sdk/Internal/Utilities/LocalStorageProtocol.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ protocol LocalStorageProtocol {

var ddlChecked: Bool { get set }

var deviceId: String? { get set }
var deviceId: String { get }

var sdkVersion: String? { get set }

Expand Down
2 changes: 1 addition & 1 deletion tests/common/MockLocalStorage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class MockLocalStorage: LocalStorageProtocol {

var ddlChecked: Bool = false

var deviceId: String? = nil
var deviceId: String = "AAAABBBB-CCCC-DDDD-EEEE-FFFFGGGGHHHH"

var sdkVersion: String? = nil

Expand Down
30 changes: 18 additions & 12 deletions tests/unit-tests/LocalStorageTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ class LocalStorageTests: XCTestCase {
override func setUp() {
super.setUp()

LocalStorageTests.clearTestUserDefaults()
LocalStorageTests.clearTestKeychain()
Self.clearTestUserDefaults()
Self.clearTestKeychain()
}

static let localStorageTestSuiteName = "localstorage.tests"
Expand All @@ -37,7 +37,7 @@ class LocalStorageTests: XCTestCase {
}

func testUserIdAndEmail() throws {
var localStorage = LocalStorage(userDefaults: LocalStorageTests.getTestUserDefaults())
var localStorage = LocalStorage(userDefaults: Self.getTestUserDefaults())
let userId = "zeeUserId"
let email = "[email protected]"
localStorage.userId = userId
Expand All @@ -48,7 +48,7 @@ class LocalStorageTests: XCTestCase {
}

func testAuthDataInKeychain() {
let testUserDefaults = LocalStorageTests.getTestUserDefaults()
let testUserDefaults = Self.getTestUserDefaults()
let testKeychain = IterableKeychain.init(wrapper: KeychainWrapper.init(serviceName: "test-localstorage"))

var localStorage = LocalStorage(userDefaults: testUserDefaults,
Expand Down Expand Up @@ -80,7 +80,7 @@ class LocalStorageTests: XCTestCase {
}

func testDDLChecked() throws {
var localStorage = LocalStorage(userDefaults: LocalStorageTests.getTestUserDefaults())
var localStorage = LocalStorage(userDefaults: Self.getTestUserDefaults())
localStorage.ddlChecked = true
XCTAssertTrue(localStorage.ddlChecked)

Expand All @@ -90,7 +90,7 @@ class LocalStorageTests: XCTestCase {

func testAttributionInfo() throws {
let mockDateProvider = MockDateProvider()
let localStorage = LocalStorage(userDefaults: LocalStorageTests.getTestUserDefaults())
let localStorage = LocalStorage(userDefaults: Self.getTestUserDefaults())
let attributionInfo = IterableAttributionInfo(campaignId: 1, templateId: 2, messageId: "3")
let currentDate = Date()
let expiration = Calendar.current.date(byAdding: Calendar.Component.hour, value: 24, to: currentDate)!
Expand All @@ -109,21 +109,27 @@ class LocalStorageTests: XCTestCase {
}

func testDeviceId() {
var localStorage = LocalStorage(userDefaults: LocalStorageTests.getTestUserDefaults())
let deviceId = UUID().uuidString
localStorage.deviceId = deviceId
let localStorage = LocalStorage(userDefaults: Self.getTestUserDefaults())
let deviceId = "AAAABBBB-CCCC-DDDD-EEEE-FFFFGGGGHHHH"
IterableUserDefaults(userDefaults: Self.getTestUserDefaults()).deviceId = deviceId
XCTAssertEqual(localStorage.deviceId, deviceId)
}

func testKeychainDeviceId() {
let localStorage = LocalStorage(userDefaults: Self.getTestUserDefaults())
let deviceId = IterableKeychain().deviceId
XCTAssertEqual(localStorage.deviceId, deviceId)
}

func testSdkVersion() {
var localStorage = LocalStorage(userDefaults: LocalStorageTests.getTestUserDefaults())
var localStorage = LocalStorage(userDefaults: Self.getTestUserDefaults())
let sdkVersion = "6.0.2"
localStorage.sdkVersion = sdkVersion
XCTAssertEqual(localStorage.sdkVersion, sdkVersion)
}

func testAuthToken() {
var localStorage = LocalStorage(userDefaults: LocalStorageTests.getTestUserDefaults())
var localStorage = LocalStorage(userDefaults: Self.getTestUserDefaults())
let authToken = "03.10.11"
localStorage.authToken = authToken
XCTAssertEqual(localStorage.authToken, authToken)
Expand Down Expand Up @@ -151,7 +157,7 @@ class LocalStorageTests: XCTestCase {

private func testLocalStorage<T>(saver: (LocalStorageProtocol, T) -> Void,
retriever: (LocalStorageProtocol) -> T?, value: T) where T: Equatable {
let localStorage = LocalStorage(userDefaults: LocalStorageTests.getTestUserDefaults())
let localStorage = LocalStorage(userDefaults: Self.getTestUserDefaults())
saver(localStorage, value)
let retrievedLocalStorage = LocalStorage(userDefaults: LocalStorageTests.getTestUserDefaults())
let retrieved = retriever(retrievedLocalStorage)
Expand Down
Loading