Skip to content

WIP: UPS batch update draft changes #558

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

Closed
wants to merge 1 commit into from
Closed

WIP: UPS batch update draft changes #558

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
81 changes: 60 additions & 21 deletions Sources/Implementation/DefaultDecisionService.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,26 @@ struct FeatureDecision {
}

class DefaultDecisionService: OPTDecisionService {
typealias UserProfile = OPTUserProfileService.UPProfile

private var _decisionBatchInProgress: Bool = false

var decisionBatchInProgress: Bool {
get {
return _decisionBatchInProgress
}
set {
// Only save if the value is changing from true to false
if _decisionBatchInProgress && !newValue {
saveProfile()
}
_decisionBatchInProgress = newValue
}
}

let bucketer: OPTBucketer
let userProfileService: OPTUserProfileService
private var userProfile: UserProfile?

// thread-safe lazy logger load (after HandlerRegisterService ready)
private let threadSafeLogger = ThreadSafeLogger()
Expand Down Expand Up @@ -88,16 +105,26 @@ class DefaultDecisionService: OPTDecisionService {
// ---- check if a valid variation is stored in the user profile ----
let ignoreUPS = (options ?? []).contains(.ignoreUserProfileService)

if !ignoreUPS,
let variationId = getVariationIdFromProfile(userId: userId, experimentId: experimentId),
let variation = experiment.getVariation(id: variationId) {
if !ignoreUPS {
if userProfile == nil {
userProfile = userProfileService.lookup(userId: userId)
}

if let profile = userProfile {
if let variationId = getVariationIdFromProfile(userId: userId, profile: profile, experimentId: experimentId),
let variation = experiment.getVariation(id: variationId) {
let info = LogMessage.gotVariationFromUserProfile(variation.key, experiment.key, userId)
logger.i(info)
reasons.addInfo(info)
return DecisionResponse(result: variation, reasons: reasons)
}
} else {
let info = LogMessage.unableToGetUserProfile(experiment.key, userId)
logger.i(info)
}

let info = LogMessage.gotVariationFromUserProfile(variation.key, experiment.key, userId)
logger.i(info)
reasons.addInfo(info)
return DecisionResponse(result: variation, reasons: reasons)
}

var bucketedVariation: Variation?
// ---- check if the user passes audience targeting before bucketing ----
let audienceResponse = doesMeetAudienceConditions(config: config,
Expand All @@ -118,7 +145,8 @@ class DefaultDecisionService: OPTDecisionService {
reasons.addInfo(info)
// save to user profile
if !ignoreUPS {
self.saveProfile(userId: userId, experimentId: experimentId, variationId: variation.id)
let buckerUserProfile = userProfile ?? UserProfile()
updateVariation(userId: userId, profile: buckerUserProfile, experimentId: experimentId, variationId: variation.key)
}
} else {
let info = LogMessage.userNotBucketedIntoVariation(userId)
Expand Down Expand Up @@ -454,9 +482,9 @@ class DefaultDecisionService: OPTDecisionService {
extension DefaultDecisionService {

func getVariationIdFromProfile(userId: String,
profile: UserProfile,
experimentId: String) -> String? {
if let profile = userProfileService.lookup(userId: userId),
let bucketMap = profile[UserProfileKeys.kBucketMap] as? OPTUserProfileService.UPBucketMap,
if let bucketMap = profile[UserProfileKeys.kBucketMap] as? OPTUserProfileService.UPBucketMap,
let experimentMap = bucketMap[experimentId],
let variationId = experimentMap[UserProfileKeys.kVariationId] {
return variationId
Expand All @@ -465,22 +493,33 @@ extension DefaultDecisionService {
}
}

func saveProfile(userId: String,
experimentId: String,
variationId: String) {
func updateVariation(userId: String,
profile: UserProfile,
experimentId: String,
variationId: String) {
DefaultDecisionService.upsRMWLock.sync {
var profile = self.userProfileService.lookup(userId: userId) ?? OPTUserProfileService.UPProfile()

var bucketMap = profile[UserProfileKeys.kBucketMap] as? OPTUserProfileService.UPBucketMap ?? OPTUserProfileService.UPBucketMap()
var _profile = profile
var bucketMap = _profile[UserProfileKeys.kBucketMap] as? OPTUserProfileService.UPBucketMap ?? OPTUserProfileService.UPBucketMap()
bucketMap[experimentId] = [UserProfileKeys.kVariationId: variationId]

profile[UserProfileKeys.kBucketMap] = bucketMap
profile[UserProfileKeys.kUserId] = userId
_profile[UserProfileKeys.kBucketMap] = bucketMap
_profile[UserProfileKeys.kUserId] = userId

self.userProfileService.save(userProfile: profile)
/// Update user profile
userProfile = _profile

self.logger.i(.savedVariationInUserProfile(variationId, experimentId, userId))
if !_decisionBatchInProgress {
saveProfile(userId: userId, experimentId: experimentId, variationId: variationId)
}
}
}

func saveProfile(userId: String? = nil, experimentId: String? = nil, variationId: String? = nil) {
guard let profile = userProfile else { return }

self.userProfileService.save(userProfile: profile)

self.logger.i(.savedVariationInUserProfile(variationId ?? "", experimentId ?? "", userId ?? ""))
}

}
3 changes: 2 additions & 1 deletion Sources/Optimizely+Decide/OptimizelyClient+Decide.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,13 +173,14 @@ extension OptimizelyClient {
var decisions = [String: OptimizelyDecision]()

let enabledFlagsOnly = allOptions.contains(.enabledFlagsOnly)
(decisionService as? DefaultDecisionService)?.decisionBatchInProgress = true
keys.forEach { key in
let decision = decide(user: user, key: key, options: options)
if !enabledFlagsOnly || decision.enabled {
decisions[key] = decision
}
}

(decisionService as? DefaultDecisionService)?.decisionBatchInProgress = false
return decisions
}

Expand Down
2 changes: 2 additions & 0 deletions Sources/Utils/LogMessage.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ enum LogMessage {
case failedToExtractRevenueFromEventTags(_ val: String)
case failedToExtractValueFromEventTags(_ val: String)
case gotVariationFromUserProfile(_ varKey: String, _ expKey: String, _ userId: String)
case unableToGetUserProfile(_ expKey: String, _ userId: String)
case rolloutHasNoExperiments(_ id: String)
case forcedVariationFound(_ key: String, _ userId: String)
case forcedVariationFoundButInvalid(_ key: String, _ userId: String)
Expand Down Expand Up @@ -85,6 +86,7 @@ extension LogMessage: CustomStringConvertible {
case .failedToExtractRevenueFromEventTags(let val): message = "Failed to parse revenue (\(val)) from event tags."
case .failedToExtractValueFromEventTags(let val): message = "Failed to parse value (\(val)) from event tags."
case .gotVariationFromUserProfile(let varKey, let expKey, let userId): message = "Returning previously activated variation (\(varKey)) of experiment (\(expKey)) for user (\(userId)) from user profile."
case .unableToGetUserProfile(let expKey, let userId): message = "Unable to get user profile for user (\(userId))."
case .rolloutHasNoExperiments(let id): message = "Rollout of feature (\(id)) has no experiments"
case .forcedVariationFound(let key, let userId): message = "Forced variation (\(key)) is found for user (\(userId))"
case .forcedVariationFoundButInvalid(let key, let userId): message = "Forced variation (\(key)) is found for user (\(userId)), but it's not in datafile."
Expand Down
Loading