Skip to content

fix(ats): enhance or fix ODP integration #470

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

Merged
merged 5 commits into from
Dec 16, 2022
Merged
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
6 changes: 3 additions & 3 deletions .github/workflows/swift.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ jobs:
- uses: actions/checkout@v3
- uses: maxim-lobanov/setup-xcode@v1
with:
xcode-version: 12.4
xcode-version: 14.1.0
- env:
SRCCLR_API_TOKEN: ${{ secrets.SRCCLR_API_TOKEN }}
run: |
Expand All @@ -56,7 +56,7 @@ jobs:
- uses: actions/checkout@v3
- uses: maxim-lobanov/setup-xcode@v1
with:
xcode-version: 12.4
xcode-version: 14.1.0
- id: prepare_for_release
name: Prepare for release
env:
Expand All @@ -79,7 +79,7 @@ jobs:
- uses: actions/checkout@v3
- uses: maxim-lobanov/setup-xcode@v1
with:
xcode-version: 12.4
xcode-version: 14.1.0
- name: Push to cocoapods.org
env:
GITHUB_TOKEN: ${{ secrets.CI_USER_TOKEN }}
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/unit_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ jobs:
- uses: actions/checkout@v3
- uses: maxim-lobanov/setup-xcode@v1
with:
xcode-version: 12.4
xcode-version: 14.1.0
- name: set SDK Branch if PR
if: ${{ github.event_name == 'pull_request' }}
run: |
Expand Down
12 changes: 7 additions & 5 deletions Sources/ODP/OdpEventManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,15 @@ class OdpEventManager {
data: [:])
}

func identifyUser(vuid: String, userId: String) {
func identifyUser(vuid: String, userId: String?) {
var identifiers = [Constants.ODP.keyForVuid: vuid]
if let userId = userId {
identifiers[Constants.ODP.keyForUserId] = userId
}

sendEvent(type: Constants.ODP.eventType,
action: "identified",
identifiers: [
Constants.ODP.keyForVuid: vuid,
Constants.ODP.keyForUserId: userId
],
identifiers: identifiers,
data: [:])
}

Expand Down
12 changes: 10 additions & 2 deletions Sources/ODP/OdpManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class OdpManager {
return
}

let userKey = vuidManager.isVuid(visitorId: userId) ? Constants.ODP.keyForVuid : Constants.ODP.keyForUserId
let userKey = OdpVuidManager.isVuid(userId) ? Constants.ODP.keyForVuid : Constants.ODP.keyForUserId
let userValue = userId

segmentManager.fetchQualifiedSegments(userKey: userKey,
Expand All @@ -94,7 +94,15 @@ class OdpManager {
return
}

eventManager.identifyUser(vuid: vuidManager.vuid, userId: userId)
var vuid = vuidManager.vuid
var fsUserId: String? = userId
if OdpVuidManager.isVuid(userId) {
// overwrite if userId is vuid (when userContext is created with vuid)
vuid = userId
fsUserId = nil
}

eventManager.identifyUser(vuid: vuid, userId: fsUserId)
}

/// Send an event to the ODP server.
Expand Down
7 changes: 4 additions & 3 deletions Sources/ODP/OdpVuidManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class OdpVuidManager {
self.vuid = load()
}

func makeVuid() -> String {
static var newVuid: String {
let maxLength = 32 // required by ODP server

// make sure UUIDv4 is used (not UUIDv1 or UUIDv6) since the trailing 5 chars will be truncated. See TDD for details.
Expand All @@ -36,9 +36,10 @@ class OdpVuidManager {
return vuid
}

func isVuid(visitorId: String) -> Bool {
static func isVuid(_ visitorId: String) -> Bool {
return visitorId.starts(with: "vuid_")
}

}

// MARK: - VUID Store
Expand All @@ -54,7 +55,7 @@ extension OdpVuidManager {
return oldVuid
}

let vuid = makeVuid()
let vuid = OdpVuidManager.newVuid
save(vuid: vuid)
return vuid
}
Expand Down
11 changes: 11 additions & 0 deletions Tests/OptimizelyTests-Common/OdpEventManagerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,17 @@ class OdpEventManagerTests: XCTestCase {
validateData(evt.data, customData: [:])
}

func testIdentifyUser_noApiKey_nilUserId() {
manager.identifyUser(vuid: "v1", userId: nil)

XCTAssertEqual(1, manager.eventQueue.count)
let evt = manager.eventQueue.getFirstItem()!
XCTAssertEqual("fullstack", evt.type)
XCTAssertEqual("identified", evt.action)
XCTAssertEqual(["vuid": "v1"], evt.identifiers)
validateData(evt.data, customData: [:])
}

func testSendEvent_apiKey() {
odpConfig = OdpConfig()
_ = odpConfig.update(apiKey: "valid", apiHost: "host", segmentsToCheck: [])
Expand Down
37 changes: 25 additions & 12 deletions Tests/OptimizelyTests-Common/OdpManagerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ class OdpManagerTests: XCTestCase {
// MARK: - registerVuid

func testRegisterVUIDCalledAutomatically() {
XCTAssertEqual(eventManager.receivedVuid, manager.vuid, "registerVUID is implicitly called on OdpManager init")
XCTAssertEqual(eventManager.receivedRegisterVuid, manager.vuid, "registerVUID is implicitly called on OdpManager init")
}

func testRegisterVUIDCalledAutomatically_odpDisabled() {
Expand All @@ -116,36 +116,47 @@ class OdpManagerTests: XCTestCase {
segmentManager: segmentManager,
eventManager: newEventManager)

XCTAssertNil(newEventManager.receivedVuid, "registerVUID should not implicitly called when ODP disabled")
XCTAssertNil(newEventManager.receivedRegisterVuid, "registerVUID should not implicitly called when ODP disabled")
}

// MARK: - identifyUser

func testIdentifyUser_datafileNotReady() {
manager.identifyUser(userId: "user-1")

XCTAssertEqual(eventManager.receivedUserId, "user-1")
XCTAssertEqual(eventManager.receivedIdentifyUserId, "user-1")
}

func testIdentifyUser_odpIntegrated() {
manager.updateOdpConfig(apiKey: "key-1", apiHost: "host-1", segmentsToCheck: [])
manager.identifyUser(userId: "user-1")

XCTAssertEqual(eventManager.receivedUserId, "user-1")
XCTAssert(OdpVuidManager.isVuid(eventManager.receivedIdentifyVuid))
XCTAssertEqual(eventManager.receivedIdentifyUserId, "user-1")
}

func testIdentifyUser_odpIntegrated_vuidAsUserId() {
manager.updateOdpConfig(apiKey: "key-1", apiHost: "host-1", segmentsToCheck: [])

let vuidAsUserId = OdpVuidManager.newVuid
manager.identifyUser(userId: vuidAsUserId)

XCTAssertEqual(eventManager.receivedIdentifyVuid, vuidAsUserId)
XCTAssertNil(eventManager.receivedIdentifyUserId)
}

func testIdentifyUser_odpNotIntegrated() {
manager.updateOdpConfig(apiKey: nil, apiHost: nil, segmentsToCheck: [])
manager.identifyUser(userId: "user-1")

XCTAssertNil(eventManager.receivedUserId, "identifyUser event requeut should be discarded if ODP not integrated.")
XCTAssertNil(eventManager.receivedIdentifyUserId, "identifyUser event requeut should be discarded if ODP not integrated.")
}

func testIdentifyUser_odpDisabled() {
manager.enabled = false
manager.identifyUser(userId: "user-1")

XCTAssertNil(eventManager.receivedUserId, "identifyUser event requeut should be discarded if ODP disabled.")
XCTAssertNil(eventManager.receivedIdentifyUserId, "identifyUser event requeut should be discarded if ODP disabled.")
}

// MARK: - sendEvent
Expand Down Expand Up @@ -315,9 +326,11 @@ class OdpManagerTests: XCTestCase {
// MARK: - Helpers

class MockOdpEventManager: OdpEventManager {
var receivedVuid: String!
var receivedUserId: String!
var receivedRegisterVuid: String!

var receivedIdentifyVuid: String!
var receivedIdentifyUserId: String?

var receivedType: String!
var receivedAction: String!
var receivedIdentifiers: [String: String]!
Expand All @@ -328,12 +341,12 @@ class OdpManagerTests: XCTestCase {
var resetCalled = false

override func registerVUID(vuid: String) {
self.receivedVuid = vuid
self.receivedRegisterVuid = vuid
}

override func identifyUser(vuid: String, userId: String) {
self.receivedVuid = vuid
self.receivedUserId = userId
override func identifyUser(vuid: String, userId: String?) {
self.receivedIdentifyVuid = vuid
self.receivedIdentifyUserId = userId
}

override func sendEvent(type: String, action: String, identifiers: [String: String], data: [String: Any?]) {
Expand Down
14 changes: 7 additions & 7 deletions Tests/OptimizelyTests-Common/OdpVuidManagerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,17 @@ import XCTest
class OdpVuidManagerTests: XCTestCase {
var manager = OdpVuidManager()

func testMakeVuid() {
let vuid = manager.makeVuid()
func testNewVuid() {
let vuid = OdpVuidManager.newVuid

XCTAssertTrue(vuid.starts(with: "vuid_"))
XCTAssertEqual(vuid.count, 32)
}

func testIsVuid() {
XCTAssertTrue(manager.isVuid(visitorId: "vuid_123"))
XCTAssertFalse(manager.isVuid(visitorId: "vuid-123"))
XCTAssertFalse(manager.isVuid(visitorId: "123"))
XCTAssertTrue(OdpVuidManager.isVuid("vuid_123"))
XCTAssertFalse(OdpVuidManager.isVuid("vuid-123"))
XCTAssertFalse(OdpVuidManager.isVuid("123"))
}

func testAutoSaveAndLoad() {
Expand All @@ -42,8 +42,8 @@ class OdpVuidManagerTests: XCTestCase {
let vuid2 = manager.vuid

XCTAssertTrue(vuid1 == vuid2)
XCTAssert(manager.isVuid(visitorId: vuid1))
XCTAssert(manager.isVuid(visitorId: vuid2))
XCTAssert(OdpVuidManager.isVuid(vuid1))
XCTAssert(OdpVuidManager.isVuid(vuid2))

UserDefaults.standard.removeObject(forKey: "optimizely-vuid")

Expand Down