Skip to content

fix: sync init flow changed to support dynamic update #297

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

Merged
merged 5 commits into from
Mar 10, 2020
Merged

fix: sync init flow changed to support dynamic update #297

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
14fba7f
fix init sync flow for datafile cache
jaeopt Feb 13, 2020
ef8d6f8
add periodicDownloadInterval prop to decouple from OPTDatafileHandler
jaeopt Feb 13, 2020
d67d3ec
fix test for init
jaeopt Feb 14, 2020
8d9f706
fix pollingEnabled check
jaeopt Feb 25, 2020
8500883
Merge branch 'master' into jae/init-sync-flow
thomaszurkan-optimizely Mar 10, 2020
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
8 changes: 8 additions & 0 deletions OptimizelySwiftSDK.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,8 @@
6E34A647231ED28600BAE302 /* empty_datafile_new_account_id.json in Resources */ = {isa = PBXBuildFile; fileRef = 6E34A63D231ED28600BAE302 /* empty_datafile_new_account_id.json */; };
6E34A648231ED28600BAE302 /* empty_datafile_new_account_id.json in Resources */ = {isa = PBXBuildFile; fileRef = 6E34A63D231ED28600BAE302 /* empty_datafile_new_account_id.json */; };
6E34A649231ED28600BAE302 /* empty_datafile_new_account_id.json in Resources */ = {isa = PBXBuildFile; fileRef = 6E34A63D231ED28600BAE302 /* empty_datafile_new_account_id.json */; };
6E5AB69323F6130D007A82B1 /* OptimizelyClientTests_Init_Sync.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6E5AB69123F6130C007A82B1 /* OptimizelyClientTests_Init_Sync.swift */; };
6E5AB69423F6130D007A82B1 /* OptimizelyClientTests_Init_Async.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6E5AB69223F6130D007A82B1 /* OptimizelyClientTests_Init_Async.swift */; };
6E614DD621E3F38A005982A1 /* Optimizely.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 6E614DCD21E3F389005982A1 /* Optimizely.framework */; };
6E636B912236C91F00AF3CEF /* Optimizely.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 6EBAEB6C21E3FEF800D13AA9 /* Optimizely.framework */; };
6E636BA02236C96700AF3CEF /* Optimizely.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 6EBAEB6C21E3FEF800D13AA9 /* Optimizely.framework */; };
Expand Down Expand Up @@ -1268,6 +1270,8 @@
6E34A623231ED04900BAE302 /* empty_datafile_new_project_id.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = empty_datafile_new_project_id.json; sourceTree = "<group>"; };
6E34A624231ED04900BAE302 /* empty_datafile_new_revision.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = empty_datafile_new_revision.json; sourceTree = "<group>"; };
6E34A63D231ED28600BAE302 /* empty_datafile_new_account_id.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = empty_datafile_new_account_id.json; sourceTree = "<group>"; };
6E5AB69123F6130C007A82B1 /* OptimizelyClientTests_Init_Sync.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = OptimizelyClientTests_Init_Sync.swift; sourceTree = "<group>"; };
6E5AB69223F6130D007A82B1 /* OptimizelyClientTests_Init_Async.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = OptimizelyClientTests_Init_Async.swift; sourceTree = "<group>"; };
6E614DCD21E3F389005982A1 /* Optimizely.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Optimizely.framework; sourceTree = BUILT_PRODUCTS_DIR; };
6E614DD521E3F38A005982A1 /* OptimizelyTests-tvOS.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = "OptimizelyTests-tvOS.xctest"; sourceTree = BUILT_PRODUCTS_DIR; };
6E636B8C2236C91F00AF3CEF /* OptimizelyTests-APIs-iOS.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = "OptimizelyTests-APIs-iOS.xctest"; sourceTree = BUILT_PRODUCTS_DIR; };
Expand Down Expand Up @@ -1911,6 +1915,8 @@
6E7519B922C5211100B2B157 /* OptimizelyTests-APIs */ = {
isa = PBXGroup;
children = (
6E5AB69223F6130D007A82B1 /* OptimizelyClientTests_Init_Async.swift */,
6E5AB69123F6130C007A82B1 /* OptimizelyClientTests_Init_Sync.swift */,
6E7519BA22C5211100B2B157 /* OptimizelyClientTests_Evaluation.swift */,
6E7519BB22C5211100B2B157 /* OptimizelyClientTests_DatafileHandler.swift */,
6E7519BC22C5211100B2B157 /* OptimizelyErrorTests.swift */,
Expand Down Expand Up @@ -2848,9 +2854,11 @@
6E7517F022C520D400B2B157 /* DataStoreMemory.swift in Sources */,
6E9B11D922C548A200C22D81 /* OptimizelyClientTests_Invalid.swift in Sources */,
6E9B11D522C548A200C22D81 /* OptimizelyClientTests_Evaluation.swift in Sources */,
6E5AB69423F6130D007A82B1 /* OptimizelyClientTests_Init_Async.swift in Sources */,
6E9B11DA22C548A200C22D81 /* OptimizelyClientTests_ObjcAPIs.m in Sources */,
6E75179A22C520D400B2B157 /* DataStoreQueueStackImpl+Extension.swift in Sources */,
6E75182022C520D400B2B157 /* BatchEventBuilder.swift in Sources */,
6E5AB69323F6130D007A82B1 /* OptimizelyClientTests_Init_Sync.swift in Sources */,
6E75184422C520D400B2B157 /* Event.swift in Sources */,
6E75194022C520D500B2B157 /* OPTDecisionService.swift in Sources */,
6E7518E022C520D400B2B157 /* ConditionLeaf.swift in Sources */,
Expand Down
49 changes: 27 additions & 22 deletions Sources/Implementation/DefaultDatafileHandler.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
/****************************************************************************
* Copyright 2019, Optimizely, Inc. and contributors *
* *
* Licensed under the Apache License, Version 2.0 (the "License"); *
* you may not use this file except in compliance with the License. *
* You may obtain a copy of the License at *
* *
* http://www.apache.org/licenses/LICENSE-2.0 *
* *
* Unless required by applicable law or agreed to in writing, software *
* distributed under the License is distributed on an "AS IS" BASIS, *
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. *
* See the License for the specific language governing permissions and *
* limitations under the License. *
***************************************************************************/
* Copyright 2019-2020, Optimizely, Inc. and contributors *
* *
* Licensed under the Apache License, Version 2.0 (the "License"); *
* you may not use this file except in compliance with the License. *
* You may obtain a copy of the License at *
* *
* http://www.apache.org/licenses/LICENSE-2.0 *
* *
* Unless required by applicable law or agreed to in writing, software *
* distributed under the License is distributed on an "AS IS" BASIS, *
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. *
* See the License for the specific language governing permissions and *
* limitations under the License. *
***************************************************************************/

import Foundation

Expand Down Expand Up @@ -84,7 +84,6 @@ class DefaultDatafileHandler: OPTDatafileHandler {
}

return request

}

open func getResponseData(sdkKey: String, response: HTTPURLResponse, url: URL?) -> Data? {
Expand All @@ -101,7 +100,8 @@ class DefaultDatafileHandler: OPTDatafileHandler {
}

open func downloadDatafile(sdkKey: String,
resourceTimeoutInterval: Double? = nil,
returnCacheIfNoChange: Bool,
resourceTimeoutInterval: Double?,
completionHandler: @escaping DatafileDownloadCompletionHandler) {

downloadQueue.async {
Expand All @@ -121,7 +121,16 @@ class DefaultDatafileHandler: OPTDatafileHandler {
result = .success(data)
} else if response.statusCode == 304 {
self.logger.d("The datafile was not modified and won't be downloaded again")
result = .success(nil)

if returnCacheIfNoChange {
if let data = self.loadSavedDatafile(sdkKey: sdkKey) {
result = .success(data)
} else {
result = .failure(.datafileLoadingFailed(sdkKey))
}
} else {
result = .success(nil)
}
}
}

Expand Down Expand Up @@ -169,7 +178,6 @@ class DefaultDatafileHandler: OPTDatafileHandler {
self.performPerodicDownload(sdkKey: sdkKey, startTime: startDate, updateInterval: updateInterval, datafileChangeNotification: datafileChangeNotification)
}
timer.invalidate()

}

func hasPeriodUpdates(sdkKey: String) -> Bool {
Expand Down Expand Up @@ -220,7 +228,6 @@ class DefaultDatafileHandler: OPTDatafileHandler {
timer.timer?.invalidate()
timers.removeValue(forKey: sdkKey)
}

}
}

Expand Down Expand Up @@ -293,7 +300,6 @@ class DefaultDatafileHandler: OPTDatafileHandler {
try? FileManager.default.removeItem(at: fileURL)
}
}

}
}

Expand All @@ -313,11 +319,10 @@ extension URLRequest {
addValue(lastModified, forHTTPHeaderField: "If-Modified-Since")
}
}

func getLastModified() -> String? {
return value(forHTTPHeaderField: "If-Modified-Since")
}

}

extension HTTPURLResponse {
Expand Down
22 changes: 21 additions & 1 deletion Sources/Optimizely/OptimizelyClient+ObjC.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,30 @@ extension OptimizelyClient {
/// - doFetchDatafileBackground: This is for debugging purposes when
/// you don't want to download the datafile. In practice, you should allow the
/// background thread to update the cache copy (optional)
public func objcStart(datafile: Data, doFetchDatafileBackground: Bool = true) throws {
public func objcStart(datafile: Data, doFetchDatafileBackground: Bool) throws {
try self.start(datafile: datafile, doFetchDatafileBackground: doFetchDatafileBackground)
}

@available(swift, obsoleted: 1.0)
@objc(startWithDatafile:doUpdateConfigOnNewDatafile:doFetchDatafileBackground:error:)
/// Start Optimizely SDK (Synchronous)
///
/// - Parameters:
/// - datafile: This datafile will be used when cached copy is not available (fresh start)
/// A cached copy from previous download is used if it's available.
/// The datafile will be updated from the server in the background thread.
/// - doUpdateConfigOnNewDatafile: When a new datafile is fetched from the server in the background thread,
/// the SDK will be updated with the new datafile immediately if this value is set to true.
/// When it's set to false (default), the new datafile is cached and will be used when the SDK is started again.
/// - doFetchDatafileBackground: This is for debugging purposes when
/// you don't want to download the datafile. In practice, you should allow the
/// background thread to update the cache copy (optional)
public func objcStart(datafile: Data, doUpdateConfigOnNewDatafile: Bool, doFetchDatafileBackground: Bool) throws {
try self.start(datafile: datafile,
doUpdateConfigOnNewDatafile: doUpdateConfigOnNewDatafile,
doFetchDatafileBackground: doFetchDatafileBackground)
}

@available(swift, obsoleted: 1.0)
@objc(activateWithExperimentKey:userId:attributes:error:)
/// Try to activate an experiment based on the experiment key and user ID with user attributes.
Expand Down
125 changes: 67 additions & 58 deletions Sources/Optimizely/OptimizelyClient.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ open class OptimizelyClient: NSObject {
// MARK: - Properties

var sdkKey: String

private var atomicConfig: AtomicProperty<ProjectConfig> = AtomicProperty<ProjectConfig>()
var config: ProjectConfig? {
get {
Expand All @@ -39,6 +40,14 @@ open class OptimizelyClient: NSObject {
}

let eventLock = DispatchQueue(label: "com.optimizely.client")

private var isPeriodicPollingEnabled: Bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: can we make this an extension?

if let handler = datafileHandler as? DefaultDatafileHandler {
return handler.hasPeriodUpdates(sdkKey: sdkKey)
} else {
return false
}
}

// MARK: - Customizable Services

Expand All @@ -54,8 +63,8 @@ open class OptimizelyClient: NSObject {
return HandlerRegistryService.shared.injectDecisionService(sdkKey: self.sdkKey)!
}

public var datafileHandler: OPTDatafileHandler {
return HandlerRegistryService.shared.injectDatafileHandler(sdkKey: self.sdkKey)!
public var datafileHandler: OPTDatafileHandler? {
return HandlerRegistryService.shared.injectDatafileHandler(sdkKey: self.sdkKey)
}

public var notificationCenter: OPTNotificationCenter? {
Expand Down Expand Up @@ -106,18 +115,22 @@ open class OptimizelyClient: NSObject {
/// - resourceTimeout: timeout for datafile download (optional)
/// - completion: callback when initialization is completed
public func start(resourceTimeout: Double? = nil, completion: ((OptimizelyResult<Data>) -> Void)? = nil) {
fetchDatafileBackground(resourceTimeout: resourceTimeout) { result in
datafileHandler?.downloadDatafile(sdkKey: sdkKey, returnCacheIfNoChange: true) { result in
switch result {
case .failure:
completion?(result)
case .success(let datafile):
guard let datafile = datafile else {
completion?(.failure(.datafileLoadingFailed(self.sdkKey)))
return
}

do {
try self.configSDK(datafile: datafile)

completion?(result)
completion?(.success(datafile))
} catch {
completion?(.failure(error as! OptimizelyError))
}
case .failure(let error):
completion?(.failure(error))
}
}
}
Expand All @@ -139,80 +152,76 @@ open class OptimizelyClient: NSObject {
/// - datafile: This datafile will be used when cached copy is not available (fresh start)
/// A cached copy from previous download is used if it's available.
/// The datafile will be updated from the server in the background thread.
/// - doUpdateConfigOnNewDatafile: When a new datafile is fetched from the server in the background thread,
/// the SDK will be updated with the new datafile immediately if this value is set to true.
/// When it's set to false (default), the new datafile is cached and will be used when the SDK is started again.
/// - doFetchDatafileBackground: This is for debugging purposes when
/// you don't want to download the datafile. In practice, you should allow the
/// background thread to update the cache copy (optional)
public func start(datafile: Data, doFetchDatafileBackground: Bool = true) throws {
let cachedDatafile = self.datafileHandler.loadSavedDatafile(sdkKey: self.sdkKey)
public func start(datafile: Data,
doUpdateConfigOnNewDatafile: Bool = false,
Copy link
Contributor

@thomaszurkan-optimizely thomaszurkan-optimizely Feb 18, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

These two parameters don't really make sense here. If you have doUpdateConfigOnNewDatafile as true but you have doFeatchDatafileBackground as false. It is an invalid combination. Maybe an enum might be better here with all valid combinations. Also, the default combination is probably not ideal if you have polling enabled which is not addressed in this combo or PR. So, you might want combos that include polling and what to do there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it's not so clean as it should be. The reason I keep "doFetchDatafileBackground" is for API backward compatibility only, where we had this option for debugging support. We can let them override "doUpdateConfigOnNewDatafile" when they still want to disable fetch completely for debugging purpose.

doFetchDatafileBackground: Bool = true) throws {
let cachedDatafile = datafileHandler?.loadSavedDatafile(sdkKey: self.sdkKey)
let selectedDatafile = cachedDatafile ?? datafile

try configSDK(datafile: selectedDatafile)

// continue to fetch updated datafile from the server in background and cache it for next sessions
if doFetchDatafileBackground { fetchDatafileBackground() }

if !doFetchDatafileBackground { return }

datafileHandler?.downloadDatafile(sdkKey: sdkKey, returnCacheIfNoChange: false) { result in
// override to update always if periodic datafile polling is enabled
// this is necessary for the case that the first cache download gets the updated datafile
guard doUpdateConfigOnNewDatafile || self.isPeriodicPollingEnabled else { return }

if case .success(let data) = result, let datafile = data {
// new datafile came in
self.updateConfigFromBackgroundFetch(data: datafile)
}
}
}

func configSDK(datafile: Data) throws {
do {
self.config = try ProjectConfig(datafile: datafile)

datafileHandler.startUpdates(sdkKey: self.sdkKey) { data in
// new datafile came in...
if let config = try? ProjectConfig(datafile: data) {
do {
if let users = self.config?.whitelistUsers {
config.whitelistUsers = users
}

self.config = config

// call reinit on the services we know we are reinitializing.

for component in HandlerRegistryService.shared.lookupComponents(sdkKey: self.sdkKey) ?? [] {
HandlerRegistryService.shared.reInitializeComponent(service: component, sdkKey: self.sdkKey)
}

}

self.sendDatafileChangeNotification(data: data)
}

datafileHandler?.startUpdates(sdkKey: self.sdkKey) { data in
// new datafile came in
self.updateConfigFromBackgroundFetch(data: data)
}
} catch {
} catch let error as OptimizelyError {
// .datafileInvalid
// .datafaileVersionInvalid
// .datafaileLoadingFailed
self.logger.e(error)
throw error
} catch {
self.logger.e(error.localizedDescription)
throw error
}
}

func fetchDatafileBackground(resourceTimeout: Double? = nil, completion: ((OptimizelyResult<Data>) -> Void)? = nil) {
func updateConfigFromBackgroundFetch(data: Data) {
guard let config = try? ProjectConfig(datafile: data) else {
return
}

datafileHandler.downloadDatafile(sdkKey: self.sdkKey, resourceTimeoutInterval: resourceTimeout) { result in
var fetchResult: OptimizelyResult<Data>

switch result {
case .failure(let error):
fetchResult = .failure(error)
case .success(let datafile):
// we got a new datafile.
if let datafile = datafile {
fetchResult = .success(datafile)
}
// we got a success but no datafile 304. So, load the saved datafile.
else if let data = self.datafileHandler.loadSavedDatafile(sdkKey: self.sdkKey) {
fetchResult = .success(data)
}
// if that fails, we have a problem.
else {
fetchResult = .failure(.datafileLoadingFailed(self.sdkKey))
}

}

completion?(fetchResult)
if let users = self.config?.whitelistUsers {
config.whitelistUsers = users
}

self.config = config

// call reinit on the services we know we are reinitializing.

for component in HandlerRegistryService.shared.lookupComponents(sdkKey: self.sdkKey) ?? [] {
HandlerRegistryService.shared.reInitializeComponent(service: component, sdkKey: self.sdkKey)
}

self.sendDatafileChangeNotification(data: data)
}

/**
* Use the activate method to start an experiment.
*
Expand Down Expand Up @@ -836,7 +845,7 @@ extension OptimizelyClient {
extension OptimizelyClient {

public func close() {
datafileHandler.stopUpdates(sdkKey: sdkKey)
datafileHandler?.stopUpdates(sdkKey: sdkKey)
eventLock.sync {}
eventDispatcher?.close()
}
Expand Down
Loading