Skip to content

Commit bf7606f

Browse files
thomaszurkan-optimizelythomaszurkan-optimizely
authored
(fix): Timer fires on main thread, datafile handler should not pass last modified if no cached datafile (#219)
* offload timer fired events * add a test for last modified if datafile not in cache * added another test with cache file existing. * don't pass in last modified if don't have saved file * add optimizelyClient level test for getting file from cache * slight refactor * cleanup lint warnings and switch to closure log message.
1 parent fabc77c commit bf7606f

File tree

8 files changed

+315
-19
lines changed

8 files changed

+315
-19
lines changed

‎OptimizelySDK/Customization/DefaultEventDispatcher.swift

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -238,13 +238,16 @@ open class DefaultEventDispatcher: BackgroundingCallbacks, OPTEventDispatcher {
238238
guard self.timer.property == nil else { return }
239239

240240
self.timer.property = Timer.scheduledTimer(withTimeInterval: self.timerInterval, repeats: true) { (timer) in
241-
if self.dataStore.count == 0 {
242-
self.timer.performAtomic { (timer) in
243-
timer.invalidate()
241+
self.dispatcher.async {
242+
if self.dataStore.count == 0 {
243+
self.timer.performAtomic() { (timer) in
244+
timer.invalidate()
245+
}
246+
self.timer.property = nil
247+
}
248+
else {
249+
self.flushEvents()
244250
}
245-
self.timer.property = nil
246-
} else {
247-
self.flushEvents()
248251
}
249252
}
250253
}

‎OptimizelySDK/Implementation/DefaultDatafileHandler.swift

Lines changed: 41 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,17 @@
1717
import Foundation
1818

1919
class DefaultDatafileHandler: OPTDatafileHandler {
20+
// endpoint used to get the datafile. This is settable after you create a OptimizelyClient instance.
2021
public var endPointStringFormat = "https://cdn.optimizely.com/datafiles/%@.json"
22+
23+
// lazy load the logger from the logger factory.
2124
lazy var logger = OPTLoggerFactory.getLogger()
25+
// the timers for all sdk keys are atomic to allow for thread access.
2226
var timers: AtomicProperty<[String:(timer: Timer?, interval: Int)]> = AtomicProperty(property: [String: (Timer?, Int)]())
27+
// we will use a simple user defaults datastore
2328
let dataStore = DataStoreUserDefaults()
24-
25-
let downloadQueue = DispatchQueue(label: "DefaultDatafileHandlerQueue", qos: DispatchQoS.default, attributes: DispatchQueue.Attributes.concurrent, autoreleaseFrequency: DispatchQueue.AutoreleaseFrequency.inherit, target: nil)
29+
// and our download queue to speed things up.
30+
let downloadQueue = DispatchQueue(label: "DefaultDatafileHandlerQueue")
2631

2732
required init() {
2833

@@ -74,8 +79,8 @@ class DefaultDatafileHandler: OPTDatafileHandler {
7479

7580
var request = URLRequest(url: url)
7681

77-
if let lastModified = dataStore.getItem(forKey: "OPTLastModified-" + sdkKey) {
78-
request.addValue(lastModified as! String, forHTTPHeaderField: "If-Modified-Since")
82+
if let lastModified = dataStore.getLastModified(sdkKey: sdkKey), isDatafileSaved(sdkKey: sdkKey) {
83+
request.setLastModified(lastModified: lastModified)
7984
}
8085

8186
return request
@@ -84,13 +89,10 @@ class DefaultDatafileHandler: OPTDatafileHandler {
8489

8590
open func getResponseData(sdkKey: String, response: HTTPURLResponse, url: URL?) -> Data? {
8691
if let url = url, let data = try? Data(contentsOf: url) {
87-
if let str = String(data: data, encoding: .utf8) {
88-
self.logger.d(str)
89-
}
92+
self.logger.d { String(data: data, encoding: .utf8) ?? "" }
9093
self.saveDatafile(sdkKey: sdkKey, dataFile: data)
91-
if let lastModified = response.allHeaderFields["Last-Modified"] {
92-
self.dataStore.saveItem(forKey: "OPTLastModified-" + sdkKey, value: lastModified)
93-
}
94+
if let lastModified = response.getLastModified() {
95+
self.dataStore.setLastModified(sdkKey: sdkKey, lastModified: lastModified) }
9496

9597
return data
9698
}
@@ -317,3 +319,32 @@ class DefaultDatafileHandler: OPTDatafileHandler {
317319

318320
}
319321
}
322+
323+
extension DataStoreUserDefaults {
324+
func getLastModified(sdkKey: String) -> String? {
325+
return getItem(forKey: "OPTLastModified-" + sdkKey) as? String
326+
}
327+
328+
func setLastModified(sdkKey: String, lastModified: String) {
329+
saveItem(forKey: "OPTLastModified-" + sdkKey, value: lastModified)
330+
}
331+
}
332+
333+
extension URLRequest {
334+
mutating func setLastModified(lastModified: String?) {
335+
if let lastModified = lastModified {
336+
addValue(lastModified, forHTTPHeaderField: "If-Modified-Since")
337+
}
338+
}
339+
340+
func getLastModified() -> String? {
341+
return value(forHTTPHeaderField: "If-Modified-Since")
342+
}
343+
344+
}
345+
346+
extension HTTPURLResponse {
347+
func getLastModified() -> String? {
348+
return allHeaderFields["Last-Modified"] as? String
349+
}
350+
}

‎OptimizelySDK/Optimizely/OptimizelyClient.swift

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -195,9 +195,6 @@ open class OptimizelyClient: NSObject {
195195

196196
func fetchDatafileBackground(resourceTimeout: Double? = nil, completion: ((OptimizelyResult<Data>) -> Void)?=nil) {
197197

198-
// TODO: fix downloadDatafile to throw OptimizelyError
199-
// those errors propagated instead of handling here
200-
201198
datafileHandler.downloadDatafile(sdkKey: self.sdkKey, resourceTimeoutInterval: resourceTimeout) { result in
202199
var fetchResult: OptimizelyResult<Data>
203200

‎OptimizelySDK/OptimizelySwiftSDK.xcodeproj/project.pbxproj

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,17 @@
108108
0B7B9EC421F2ACB100056589 /* DataStoreFile.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0B7B9EC221F2ACB100056589 /* DataStoreFile.swift */; };
109109
0B7B9EC521F51AF600056589 /* DataStoreFile.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0B7B9EC221F2ACB100056589 /* DataStoreFile.swift */; };
110110
0B7B9EC621F51AF700056589 /* DataStoreFile.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0B7B9EC221F2ACB100056589 /* DataStoreFile.swift */; };
111+
0B86665A22A82FB4009CD211 /* MockUrlSession.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0B86665922A82FB4009CD211 /* MockUrlSession.swift */; };
112+
0B86665B22A82FB4009CD211 /* MockUrlSession.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0B86665922A82FB4009CD211 /* MockUrlSession.swift */; };
113+
0B86665C22A82FB4009CD211 /* MockUrlSession.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0B86665922A82FB4009CD211 /* MockUrlSession.swift */; };
114+
0B86665D22A82FB4009CD211 /* MockUrlSession.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0B86665922A82FB4009CD211 /* MockUrlSession.swift */; };
115+
0B86665E22A82FB4009CD211 /* MockUrlSession.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0B86665922A82FB4009CD211 /* MockUrlSession.swift */; };
116+
0B86665F22A82FB4009CD211 /* MockUrlSession.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0B86665922A82FB4009CD211 /* MockUrlSession.swift */; };
117+
0B86666022A82FB4009CD211 /* MockUrlSession.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0B86665922A82FB4009CD211 /* MockUrlSession.swift */; };
118+
0B86666122A82FB4009CD211 /* MockUrlSession.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0B86665922A82FB4009CD211 /* MockUrlSession.swift */; };
119+
0B86666222A82FB4009CD211 /* MockUrlSession.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0B86665922A82FB4009CD211 /* MockUrlSession.swift */; };
120+
0B86666322A82FB4009CD211 /* MockUrlSession.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0B86665922A82FB4009CD211 /* MockUrlSession.swift */; };
121+
0B86666522A83EFC009CD211 /* OptimizelyClientTests_DatafileHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0B86666422A83EFC009CD211 /* OptimizelyClientTests_DatafileHandler.swift */; };
111122
0B8F537E223FFB0500C56082 /* OptimizelyClientTests_Group.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0B8F537B223FFB0500C56082 /* OptimizelyClientTests_Group.swift */; };
112123
0BAB9AFE22567E21000DC388 /* EventDispatcherTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0B894EBB224163FF004A2ADB /* EventDispatcherTests.swift */; };
113124
0BAB9AFF22567E24000DC388 /* EventDispatcherTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0B894EBB224163FF004A2ADB /* EventDispatcherTests.swift */; };
@@ -1216,6 +1227,8 @@
12161227
0B807B1021C2DD4E0090DDC8 /* BatchEventBuilder.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BatchEventBuilder.swift; sourceTree = "<group>"; };
12171228
0B807B1521C312390090DDC8 /* DefaultEventDispatcher.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DefaultEventDispatcher.swift; sourceTree = "<group>"; };
12181229
0B807B1B21C44E000090DDC8 /* DefaultDatafileHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DefaultDatafileHandler.swift; sourceTree = "<group>"; };
1230+
0B86665922A82FB4009CD211 /* MockUrlSession.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockUrlSession.swift; sourceTree = "<group>"; };
1231+
0B86666422A83EFC009CD211 /* OptimizelyClientTests_DatafileHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OptimizelyClientTests_DatafileHandler.swift; sourceTree = "<group>"; };
12191232
0B894EBB224163FF004A2ADB /* EventDispatcherTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EventDispatcherTests.swift; sourceTree = "<group>"; };
12201233
0B8F537B223FFB0500C56082 /* OptimizelyClientTests_Group.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OptimizelyClientTests_Group.swift; sourceTree = "<group>"; };
12211234
0BC47BD52242F7EF00E5C2CD /* DatafileHandlerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DatafileHandlerTests.swift; sourceTree = "<group>"; };
@@ -1735,6 +1748,7 @@
17351748
6E8CB5C8224C461A00B8CB7A /* OptimizelyClientTests_Variables.swift */,
17361749
0B8F537B223FFB0500C56082 /* OptimizelyClientTests_Group.swift */,
17371750
6E341E2F22615D1F005416B7 /* OptimizelyClientTests_ObjcAPIs.m */,
1751+
0B86666422A83EFC009CD211 /* OptimizelyClientTests_DatafileHandler.swift */,
17381752
);
17391753
path = "OptimizelyTests-APIs";
17401754
sourceTree = "<group>";
@@ -1824,6 +1838,7 @@
18241838
isa = PBXGroup;
18251839
children = (
18261840
6E7FC6A222121D27009A171F /* OTUtils.swift */,
1841+
0B86665922A82FB4009CD211 /* MockUrlSession.swift */,
18271842
);
18281843
path = TestUtils;
18291844
sourceTree = "<group>";
@@ -2716,6 +2731,7 @@
27162731
6E4DD89321E525FD00B0C2C7 /* DefaultNotificationCenter.swift in Sources */,
27172732
6E4DD8B221E5260700B0C2C7 /* OPTUserProfileService.swift in Sources */,
27182733
6E4DD8F821E530A900B0C2C7 /* OptimizelyError.swift in Sources */,
2734+
0B86666022A82FB4009CD211 /* MockUrlSession.swift in Sources */,
27192735
6E4DD8CF21E5261600B0C2C7 /* Event.swift in Sources */,
27202736
6E4DD8CD21E5261600B0C2C7 /* FeatureVariable.swift in Sources */,
27212737
6E4DD8B621E5260700B0C2C7 /* DataStoreQueueStack.swift in Sources */,
@@ -2809,6 +2825,7 @@
28092825
6E636BC22236C99800AF3CEF /* BatchEventBuilder.swift in Sources */,
28102826
6E636C062236C9C500AF3CEF /* OptimizelyClient+Extension.swift in Sources */,
28112827
6E636C0E2236C9CA00AF3CEF /* Constants.swift in Sources */,
2828+
0B86665C22A82FB4009CD211 /* MockUrlSession.swift in Sources */,
28122829
6E636BCC2236C9A800AF3CEF /* OPTBucketer.swift in Sources */,
28132830
6E636BDC2236C9B100AF3CEF /* EventForDispatch.swift in Sources */,
28142831
6EF4B4E5223836C1002DE8B6 /* BackgroundingCallbacks.swift in Sources */,
@@ -2819,6 +2836,7 @@
28192836
6E636C0C2236C9CA00AF3CEF /* MurmurHash3.swift in Sources */,
28202837
6E636BAE2236C98300AF3CEF /* DefaultLogger.swift in Sources */,
28212838
6E636BCD2236C9A800AF3CEF /* OPTDecisionService.swift in Sources */,
2839+
0B86666522A83EFC009CD211 /* OptimizelyClientTests_DatafileHandler.swift in Sources */,
28222840
6EF4B4E322383631002DE8B6 /* AtomicProperty.swift in Sources */,
28232841
6E636BED2236C9BC00AF3CEF /* Rollout.swift in Sources */,
28242842
6E636C0F2236C9CA00AF3CEF /* HandlerRegistryService.swift in Sources */,
@@ -2857,6 +2875,7 @@
28572875
6E636BC02236C99400AF3CEF /* DataStoreFile.swift in Sources */,
28582876
6E636BB22236C98400AF3CEF /* DefaultEventDispatcher.swift in Sources */,
28592877
6E636BAD2236C97E00AF3CEF /* OptimizelyLogLevel.swift in Sources */,
2878+
0B86665F22A82FB4009CD211 /* MockUrlSession.swift in Sources */,
28602879
6E636BE52236C9B700AF3CEF /* Audience.swift in Sources */,
28612880
6E636C072236C9C500AF3CEF /* EventForDispatch+Extension.swift in Sources */,
28622881
6E636BFB2236C9BD00AF3CEF /* FeatureFlag.swift in Sources */,
@@ -2949,6 +2968,7 @@
29492968
6EA425272218E4A300B074B5 /* DefaultEventDispatcher.swift in Sources */,
29502969
6E8CB5A3224BF4BE00B8CB7A /* DecisionServiceTests_Features.swift in Sources */,
29512970
6EA425352218E4A300B074B5 /* OPTDecisionService.swift in Sources */,
2971+
0B86666122A82FB4009CD211 /* MockUrlSession.swift in Sources */,
29522972
6E2B8D652257C5F6003C4799 /* DecisionServiceTests_Experiments.swift in Sources */,
29532973
6EA425342218E4A300B074B5 /* OPTBucketer.swift in Sources */,
29542974
6EA425232218E46C00B074B5 /* OptimizelyResult.swift in Sources */,
@@ -2991,6 +3011,7 @@
29913011
6EA425A22218E6AE00B074B5 /* AttributeValueTests_Evaluate.swift in Sources */,
29923012
6E30A071224DA4D3005EFA1D /* AudienceTests.swift in Sources */,
29933013
6EA426252218E74F00B074B5 /* DefaultDecisionService.swift in Sources */,
3014+
0B86666222A82FB4009CD211 /* MockUrlSession.swift in Sources */,
29943015
6EA07225223AC72D00F447CF /* Utils.swift in Sources */,
29953016
6EA426362218E74F00B074B5 /* ProjectConfig.swift in Sources */,
29963017
6EA425AA2218E6AE00B074B5 /* FeatureVariableTests.swift in Sources */,
@@ -3125,6 +3146,7 @@
31253146
0BDF9DC4228242E500CCD8D1 /* SDKVersion.swift in Sources */,
31263147
6EA425B02218E74D00B074B5 /* OptimizelyLogLevel.swift in Sources */,
31273148
6EA425DD2218E74D00B074B5 /* MurmurHash3.swift in Sources */,
3149+
0B86665B22A82FB4009CD211 /* MockUrlSession.swift in Sources */,
31283150
6EA425CF2218E74D00B074B5 /* Project.swift in Sources */,
31293151
6EA425BF2218E74D00B074B5 /* OPTBucketer.swift in Sources */,
31303152
6EA425DF2218E74D00B074B5 /* Constants.swift in Sources */,
@@ -3166,6 +3188,7 @@
31663188
6EA425932218E6AD00B074B5 /* AttributeValueTests_Evaluate.swift in Sources */,
31673189
6E30A070224DA4D3005EFA1D /* AudienceTests.swift in Sources */,
31683190
6EA425F12218E74E00B074B5 /* DefaultDecisionService.swift in Sources */,
3191+
0B86665D22A82FB4009CD211 /* MockUrlSession.swift in Sources */,
31693192
6EA07220223AC72D00F447CF /* Utils.swift in Sources */,
31703193
6EA426022218E74E00B074B5 /* ProjectConfig.swift in Sources */,
31713194
6EA4259B2218E6AD00B074B5 /* FeatureVariableTests.swift in Sources */,
@@ -3286,6 +3309,7 @@
32863309
6EA42687221925B100B074B5 /* Audience.swift in Sources */,
32873310
6EA426CC221925DE00B074B5 /* OptimizelyResult.swift in Sources */,
32883311
6EDE920E22273EE500840112 /* OTUtils.swift in Sources */,
3312+
0B86665E22A82FB4009CD211 /* MockUrlSession.swift in Sources */,
32893313
6EA426D1221925DE00B074B5 /* DefaultUserProfileService.swift in Sources */,
32903314
6EA426B2221925BB00B074B5 /* OPTDataStore.swift in Sources */,
32913315
6EA42692221925B100B074B5 /* Event.swift in Sources */,
@@ -3354,6 +3378,7 @@
33543378
6EA426D6221925DF00B074B5 /* OptimizelyResult.swift in Sources */,
33553379
6EDE920F22273EE600840112 /* OTUtils.swift in Sources */,
33563380
6EA426DB221925DF00B074B5 /* DefaultUserProfileService.swift in Sources */,
3381+
0B86666322A82FB4009CD211 /* MockUrlSession.swift in Sources */,
33573382
6EA426BA221925BC00B074B5 /* OPTDataStore.swift in Sources */,
33583383
6EA426A5221925B300B074B5 /* Event.swift in Sources */,
33593384
6EA426A2221925B300B074B5 /* Variable.swift in Sources */,
@@ -3473,6 +3498,7 @@
34733498
6E4DD8D421E5273F00B0C2C7 /* UserAttribute.swift in Sources */,
34743499
6E4DD8C721E5261500B0C2C7 /* Attribute.swift in Sources */,
34753500
6E4DD8D721E5274600B0C2C7 /* ConditionHolder.swift in Sources */,
3501+
0B86665A22A82FB4009CD211 /* MockUrlSession.swift in Sources */,
34763502
0B0590152209D8790007F4A2 /* ArrayEventForDispatch+Extension.swift in Sources */,
34773503
6E4F0D7B22133C5A00DD13A5 /* AttributeValue.swift in Sources */,
34783504
6E4DD8F721E530A900B0C2C7 /* OptimizelyError.swift in Sources */,

‎OptimizelySDK/OptimizelyTests/OptimizelyTests-APIs/OptimizelyClientTests_DatafileHandler.swift

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
//
2+
/****************************************************************************
3+
* Copyright 2019, Optimizely, Inc. and contributors *
4+
* *
5+
* Licensed under the Apache License, Version 2.0 (the "License"); *
6+
* you may not use this file except in compliance with the License. *
7+
* You may obtain a copy of the License at *
8+
* *
9+
* http://www.apache.org/licenses/LICENSE-2.0 *
10+
* *
11+
* Unless required by applicable law or agreed to in writing, software *
12+
* distributed under the License is distributed on an "AS IS" BASIS, *
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. *
14+
* See the License for the specific language governing permissions and *
15+
* limitations under the License. *
16+
***************************************************************************/
17+
18+
19+
import XCTest
20+
21+
class OptimizelyClientTests_DatafileHandler: XCTestCase {
22+
23+
override func setUp() {
24+
// Put setup code here. This method is called before the invocation of each test method in the class.
25+
}
26+
27+
override func tearDown() {
28+
// Put teardown code here. This method is called after the invocation of each test method in the class.
29+
}
30+
31+
func testOptimizelyClientWithCachedDatafile() {
32+
var fileUrl:URL?
33+
34+
// create a dummy file at a url to use as our datafile local download
35+
fileUrl = OTUtils.saveAFile(name: "localcdn", data: OTUtils.loadJSONDatafile("api_datafile")!)
36+
37+
// default datafile handler
38+
class InnerDatafileHandler : DefaultDatafileHandler {
39+
var localFileUrl:URL?
40+
// override getSession to return our own session.
41+
override func getSession(resourceTimeoutInterval: Double?) -> URLSession {
42+
43+
let session = MockUrlSession()
44+
session.downloadCacheUrl = localFileUrl
45+
46+
return session
47+
}
48+
}
49+
50+
// create test datafile handler
51+
let handler = InnerDatafileHandler()
52+
//save the cached datafile..
53+
let data = OTUtils.loadJSONDatafile("api_datafile")
54+
55+
handler.saveDatafile(sdkKey: "localcdnTestSDKKey", dataFile: data!)
56+
handler.dataStore.setLastModified(sdkKey: "localcdnTestSDKKey", lastModified: "1234")
57+
// set the url to use as our datafile download url
58+
handler.localFileUrl = fileUrl
59+
60+
HandlerRegistryService.shared.registerBinding(binder: Binder<OPTDatafileHandler>(service: OPTDatafileHandler.self).using(instance: handler).singetlon().sdkKey(key: "localcdnTestSDKKey").reInitializeStrategy(strategy: .reUse))
61+
62+
let client = OptimizelyClient(sdkKey: "localcdnTestSDKKey")
63+
64+
let expectation = XCTestExpectation(description: "get datafile from cache")
65+
66+
client.start() { (result) in
67+
switch result {
68+
case .success(let data):
69+
XCTAssert(!data.isEmpty)
70+
expectation.fulfill()
71+
case .failure:
72+
XCTAssert(false)
73+
}
74+
}
75+
76+
wait(for: [expectation], timeout: 3)
77+
78+
try? FileManager.default.removeItem(at: fileUrl!)
79+
80+
client.datafileHandler.stopAllUpdates()
81+
82+
HandlerRegistryService.shared.binders.removeAll()
83+
84+
// This is an example of a functional test case.
85+
// Use XCTAssert and related functions to verify your tests produce the correct results.
86+
}
87+
88+
}

0 commit comments

Comments
 (0)